Skip to content

Remove package:typed_data from package:flutter dependencies#99604

Merged
fluttergithubbot merged 4 commits into
flutter:masterfrom
jonahwilliams:fix_message_trim
Mar 7, 2022
Merged

Remove package:typed_data from package:flutter dependencies#99604
fluttergithubbot merged 4 commits into
flutter:masterfrom
jonahwilliams:fix_message_trim

Conversation

@jonahwilliams

Copy link
Copy Markdown
Contributor

Replace the usaged of the Uint8Buffer from package:typed_data with a List. This actually improves performance on wembly, using the following benchmark:

import 'package:flutter/services.dart';

ByteData? result;

void main() {
  for (var x = 0; x < 10; x++) {
    var codec = StandardMessageCodec();
    var sw = Stopwatch()..start();
    for (var i = 0; i < 100; i++) {
      result = codec.encodeMessage({
        for (var i = 0; i < 200; i++)
          i: [for (var j = 0; j < 23; j++) j]
      });
    }
    print(sw.elapsedMilliseconds);
    print(result);
  }
}

ToT:

An Observatory debugger and profiler on wembley is available at: http://127.0.0.1:61792/wKRO2rVZZf4=/
I/flutter ( 4652): 488
I/flutter ( 4652): TypedDataView(cid: 148)
I/flutter ( 4652): 451
I/flutter ( 4652): TypedDataView(cid: 148)
I/flutter ( 4652): 449
I/flutter ( 4652): TypedDataView(cid: 148)
I/flutter ( 4652): 449
I/flutter ( 4652): TypedDataView(cid: 148)
I/flutter ( 4652): 447
I/flutter ( 4652): TypedDataView(cid: 148)
I/flutter ( 4652): 448
I/flutter ( 4652): TypedDataView(cid: 148)
I/flutter ( 4652): 449
I/flutter ( 4652): TypedDataView(cid: 148)
I/flutter ( 4652): 449
I/flutter ( 4652): TypedDataView(cid: 148)
I/flutter ( 4652): 450
I/flutter ( 4652): TypedDataView(cid: 148)
I/flutter ( 4652): 448
I/flutter ( 4652): TypedDataView(cid: 148)

With Change

An Observatory debugger and profiler on wembley is available at: http://127.0.0.1:61886/wBKsj-QFjxY=/
I/flutter ( 4807): 456
I/flutter ( 4807): TypedDataView(cid: 148)
I/flutter ( 4807): 372
I/flutter ( 4807): TypedDataView(cid: 148)
I/flutter ( 4807): 363
I/flutter ( 4807): TypedDataView(cid: 148)
I/flutter ( 4807): 368
I/flutter ( 4807): TypedDataView(cid: 148)
I/flutter ( 4807): 364
I/flutter ( 4807): TypedDataView(cid: 148)
I/flutter ( 4807): 362
I/flutter ( 4807): TypedDataView(cid: 148)
I/flutter ( 4807): 363
I/flutter ( 4807): TypedDataView(cid: 148)
I/flutter ( 4807): 363
I/flutter ( 4807): TypedDataView(cid: 148)
I/flutter ( 4807): 363
I/flutter ( 4807): TypedDataView(cid: 148)
I/flutter ( 4807): 364
I/flutter ( 4807): TypedDataView(cid: 148)

@flutter-dashboard flutter-dashboard Bot added framework flutter/packages/flutter repository. See also f: labels. c: contributor-productivity Team-specific productivity, code health, technical debt. labels Mar 5, 2022
_isDone = false,
_eightBytes = ByteData(8) {
_eightBytesAsList = _eightBytes.buffer.asUint8List();
factory WriteBuffer() {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allows avoiding a late for _eightBytesAsList

collection: 1.15.0
material_color_utilities: 0.1.4
meta: 1.7.0
typed_data: 1.3.0

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to run update-packages, delaying until approved due to churn

@dnfield dnfield left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm as long as update packages is happy

I wonder how much we're saving by avoiding the late field vs the list.

@dnfield

dnfield commented Mar 5, 2022

Copy link
Copy Markdown
Contributor

Do we have some list of forbidden packages too? If so we should add this one.

@jonahwilliams

Copy link
Copy Markdown
Contributor Author

Probably not much but IMO late is best avoided if its easy to

@dnfield

dnfield commented Mar 6, 2022

Copy link
Copy Markdown
Contributor

Late requires extra runtime checks on every access

@jonahwilliams

Copy link
Copy Markdown
Contributor Author

Yup, and I'm sure it makes some difference. Though I prefer to avoid it not for performance reasons but because it more or less disables the static analysis you get for correct initialization.

@jonahwilliams jonahwilliams changed the title Remove package:typed_data from dependencies Remove package:typed_data from package:flutter dependencies Mar 6, 2022
@jonahwilliams

Copy link
Copy Markdown
Contributor Author

Turns out package:typed_data is still used elsewhere. Will follow up to see if it is easy to remove

@jonahwilliams

Copy link
Copy Markdown
Contributor Author

Requires a g3fix to update BUILD file

@fluttergithubbot

Copy link
Copy Markdown
Contributor

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@jonahwilliams

Copy link
Copy Markdown
Contributor Author

Looks like flutter_goldens_client uses package:crypto for md5 encoding which pulls in package:typed_data

@goderbauer goderbauer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

url_launcher_linux
)

list(APPEND FLUTTER_FFI_PLUGIN_LIST

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the change to this file related to this PR?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably comes from running pub get aftr the FFI plugin change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This gets auto updated whenever I run it, and it looks like its not gitignored....

Should be safe to check in :D

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 9, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 9, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 9, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 9, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 9, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 9, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 9, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 9, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 9, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 9, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 9, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 9, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 9, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 9, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 9, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 9, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants