Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ListCopy.toList.100 benchmark is 30% slower with unforked NNBD libraries #40917

Open
alexmarkov opened this issue Mar 6, 2020 · 3 comments
Open
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. NNBD Issues related to NNBD Release P3 A lower priority bug or feature request triaged Issue has been triaged by sub team

Comments

@alexmarkov
Copy link
Contributor

This regression can be reproduced with https://dart-review.googlesource.com/c/sdk/+/134808 and the following command:

dart benchmarks/ListCopy/dart/ListCopy.dart

The following changes in NNBD core libraries slowed it down:

  • _GrowableList.toList:
-  List list = growable ? new _List(length) : new _List<T>(length);
+  final list = new _List<T>(length);
  • List.filled is now called from ListMixin.toList and it starts appearing in the profile.

/cc @a-siva @mraleph @lrhn

@alexmarkov alexmarkov added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. NNBD Issues related to NNBD Release vm-nnbd-unfork-sdk Label for all issues that need to be done before the nnbd sdk can be unforked labels Mar 6, 2020
@alexmarkov alexmarkov self-assigned this Mar 6, 2020
dart-bot pushed a commit that referenced this issue Mar 10, 2020
With NNBD core libraries:
ListCopy.toList.100(RunTime) 4129 -> 2376 us.

With default core libraries:
ListCopy.toList.100(RunTime) 3006 -> 2140 us.

Issue: #40917
Change-Id: I7a319b6a150eeb5c4765c299dd45638bed2b4406
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/138720
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
@alexmarkov
Copy link
Contributor Author

7e0139e improved performance of _GrowableList.toList both for NNBD and default core libraries. The first reason for regression is fixed.

Although the benchmark ListCopy.toList.100 is now faster than before (both with NNBD and default core libraries), with NNBD core libraries the benchmark is still ~11% slower compared to default core libraries.

@a-siva
Copy link
Contributor

a-siva commented Mar 11, 2020

Is the current NNBD number in line with the old default core libraries number?

@alexmarkov
Copy link
Contributor Author

The numbers which I measured on my machine before and after 7e0139e:

With NNBD core libraries:
ListCopy.toList.100(RunTime) 4129 -> 2376 us.

With default core libraries:
ListCopy.toList.100(RunTime) 3006 -> 2140 us.

@a-siva a-siva added P3 A lower priority bug or feature request and removed vm-nnbd-unfork-sdk Label for all issues that need to be done before the nnbd sdk can be unforked labels Oct 28, 2020
@alexmarkov alexmarkov added the triaged Issue has been triaged by sub team label Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. NNBD Issues related to NNBD Release P3 A lower priority bug or feature request triaged Issue has been triaged by sub team
Projects
None yet
Development

No branches or pull requests

2 participants