-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Port classfile parsing improvements #10037
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
Conversation
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/10037/ to see the changes. Benchmarks is based on merging with master (97da3cb) |
1 similar comment
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/10037/ to see the changes. Benchmarks is based on merging with master (97da3cb) |
compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala
Outdated
Show resolved
Hide resolved
b301881
to
3c91893
Compare
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/10037/ to see the changes. Benchmarks is based on merging with master (6098ec2) |
ffd68fe
to
a1f7c7d
Compare
test performance please |
performance test scheduled: 1 job(s) in queue, 0 running. |
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/10037/ to see the changes. Benchmarks is based on merging with master (6098ec2) |
9d42245
to
1ddb130
Compare
compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala
Outdated
Show resolved
Hide resolved
1ddb130
to
42a5dd4
Compare
test performance please |
performance test scheduled: 3 job(s) in queue, 1 running. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! I suggest squashing all commits into one to keep the history easy to read here, then the commit message can provide some context:
- It should mention that this PR is based/inspired by Make ClassfileParser more efficient, lazy, and less cycle prone. [ci: last-only] scala#6653, but also say that unlike scalac we already had lazy completers
- It should mention that more work is needed to get the ClassfileParser instances to not be kept in memory longer than needed as we discussed
- Since some part of the code are directly copied from Make ClassfileParser more efficient, lazy, and less cycle prone. [ci: last-only] scala#6653, it should include a
Co-authored-by: Jason Zaugg <[email protected]>
This commit ports the following PR from Scala 2: scala/scala#6653 Unlike Scala 2, we already had lazy completers. Co-authored-by: Jason Zaugg <[email protected]>
- We need to load the type `scala.deprecated` more lazily in compiling stdlib More work is needed to get the ClassfileParser instances to not be kept in memory longer than needed. Some ideas for further refactoring: - Make `pool` pass through as `DataReader`, and make a facade for them. As both have exactly the same life duration. - Move `MemberCompleter`, `AttributeCompleter` and `InnerClass` out of `ClassfileParser`.
748d43b
to
a8f1cfb
Compare
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/10037/ to see the changes. Benchmarks is based on merging with master (ea64026) |
Port classfile parsing improvements