Skip to content

Conversation

@jonahwilliams
Copy link
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
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
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

Copy link
Contributor

@dnfield dnfield left a comment

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
Copy link
Contributor

dnfield commented Mar 5, 2022

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

@jonahwilliams
Copy link
Contributor Author

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

@dnfield
Copy link
Contributor

dnfield commented Mar 6, 2022

Late requires extra runtime checks on every access

@jonahwilliams
Copy link
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
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
Contributor Author

Requires a g3fix to update BUILD file

@fluttergithubbot
Copy link
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
Contributor Author

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

Copy link
Member

@goderbauer goderbauer left a comment

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
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
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
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 11, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 11, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 11, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 11, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 11, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 11, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 11, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 12, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 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