Manual roll of Dart with fixups to zircon system.dart #44185
Conversation
https://dart.googlesource.com/sdk.git/+log/4fd35600b477..0e02245635ea Merge to Dart Version 3.2.0-16.0.dev
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
| @@ -86,7 +86,8 @@ class ReadResult extends _Result { | |||
| /// Returns the bytes as a Uint8List. If status != OK this will throw | |||
| /// an exception. | |||
| Uint8List bytesAsUint8List() { | |||
| return _bytes!.buffer.asUint8List(_bytes!.offsetInBytes, _numBytes!); | |||
| final lbytes = _bytes!; | |||
There was a problem hiding this comment.
Use the bytes getter (which returns a non-nullable ByteData) that is already available in ReadResult and ReadEtcResult
| @@ -86,7 +86,8 @@ class ReadResult extends _Result { | |||
| /// Returns the bytes as a Uint8List. If status != OK this will throw | |||
| /// an exception. | |||
| Uint8List bytesAsUint8List() { | |||
| return _bytes!.buffer.asUint8List(_bytes!.offsetInBytes, _numBytes!); | |||
| final lbytes = bytes; | |||
| return lbytes.buffer.asUint8List(lbytes.offsetInBytes, _numBytes!); | |||
There was a problem hiding this comment.
Remove lbytes and replace it with bytes
There was a problem hiding this comment.
wouldn't that be a little less efficient with two getter calls involving null checks?
There was a problem hiding this comment.
Either is fine - not sure if it matters for performance
There was a problem hiding this comment.
Added @stereotype441 for his take on which form would be better.
There was a problem hiding this comment.
I don't have a lot to add about which form would be better. @a-siva is right that two getter calls involving null checks might in principle be slightly worse in performance. Then again, it's possible that due to inlining, the two null checks will be recognized by an optimization pass as redundant, in which case it might not matter at all. It's also possible that this is simply not a hot enough code path to have a noticeable effect.
So lgtm either way.
| @@ -86,7 +86,8 @@ class ReadResult extends _Result { | |||
| /// Returns the bytes as a Uint8List. If status != OK this will throw | |||
| /// an exception. | |||
| Uint8List bytesAsUint8List() { | |||
| return _bytes!.buffer.asUint8List(_bytes!.offsetInBytes, _numBytes!); | |||
| final lbytes = bytes; | |||
| return lbytes.buffer.asUint8List(lbytes.offsetInBytes, _numBytes!); | |||
There was a problem hiding this comment.
I don't have a lot to add about which form would be better. @a-siva is right that two getter calls involving null checks might in principle be slightly worse in performance. Then again, it's possible that due to inlining, the two null checks will be recognized by an optimization pass as redundant, in which case it might not matter at all. It's also possible that this is simply not a hot enough code path to have a noticeable effect.
So lgtm either way.
…131639) flutter/engine@fe23695...bb2a903 2023-07-31 skia-flutter-autoroll@skia.org Roll ANGLE from 196713767078 to cb4b1f3641ae (1 revision) (flutter/engine#44193) 2023-07-31 a-siva@users.noreply.github.com Manual roll of Dart with fixups to zircon system.dart (flutter/engine#44185) 2023-07-31 chinmaygarde@google.com [Impeller] Update guidance on how to run with Metal validation without Xcode. (flutter/engine#44190) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC jonahwilliams@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
…lutter#131639) flutter/engine@fe23695...bb2a903 2023-07-31 skia-flutter-autoroll@skia.org Roll ANGLE from 196713767078 to cb4b1f3641ae (1 revision) (flutter/engine#44193) 2023-07-31 a-siva@users.noreply.github.com Manual roll of Dart with fixups to zircon system.dart (flutter/engine#44185) 2023-07-31 chinmaygarde@google.com [Impeller] Update guidance on how to run with Metal validation without Xcode. (flutter/engine#44190) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC jonahwilliams@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
https://dart.googlesource.com/sdk.git/+log/4fd35600b477..0e02245635ea Merge to Dart Version 3.2.0-16.0.dev
https://dart.googlesource.com/sdk.git/+log/4fd35600b477..0e02245635ea
Merge to Dart Version 3.2.0-16.0.dev