dart-lang
`PropertyAccessorElementImpl`: optimize `name` getter to avoid calls to `considerCanonicalizeString`
The name getter is a hotspot and shouldn't resolve String canonicalization for each call.
- [✅] I’ve reviewed the contributor guide and applied the relevant portions to this PR.
Thank you for your contribution! This project uses Gerrit for code reviews. Your pull request has automatically been converted into a code review at:
https://dart-review.googlesource.com/c/sdk/+/381704
Please wait for a developer to review your code review at the above link; you can speed up the review if you sign into Gerrit and manually add a reviewer that has recently worked on the relevant code. See CONTRIBUTING.md to learn how to upload changes to Gerrit directly.
Additional commits pushed to this PR will update both the PR and the corresponding Gerrit CL. After the review is complete on the CL, your reviewer will merge the CL (automatically closing this PR).
![copybara-service[bot] avatar](https://avatars.githubusercontent.com/in/44061?v=4 "Published by a pub.dev verified publisher"){kind=small}
https://dart-review.googlesource.com/c/sdk/+/381704 has been updated with the latest commits from this pull request.
https://dart-review.googlesource.com/c/sdk/+/381704 has been updated with the latest commits from this pull request.
https://dart-review.googlesource.com/c/sdk/+/381704 has been updated with the latest commits from this pull request.
https://dart-review.googlesource.com/c/sdk/+/381704 has been updated with the latest commits from this pull request.
@scheglov reviewed this and performed some benchmarks.
I plan to run additional benchmarks to demonstrate that this optimization is significant.
The key discussion is whether PropertyAccessorElementImpl should have an additional field to bypass considerCanonicalizeString.
IMHO, given the current level of optimization in the analyzer, achieving an extra 10% improvement will likely require accumulating several smaller optimizations, each contributing around 1%.
https://dart-review.googlesource.com/c/sdk/+/381704 has been updated with the latest commits from this pull request.
https://dart-review.googlesource.com/c/sdk/+/381704 has been updated with the latest commits from this pull request.
@bwilkerson @scheglov please make a call if you want to accept or reject this PR and if you would like to accept submit corresponding CL to CQ (FWIW @bwilkerson has already voted +1).