Split Context into Period/Mode and everything else #9372
Conversation
There was a problem hiding this comment.
Hello, and thank you for opening this PR! 🎉
All contributors have signed the CLA, thank you! ❤️
Commit Messages
We want to keep history, but for that to actually be useful we have
some rules on how to format our commit messages (relevant xkcd).
Please stick to these guidelines for commit messages:
- Separate subject from body with a blank line
- When fixing an issue, start your commit message with
Fix #<ISSUE-NBR>:- Limit the subject line to 72 characters
- Capitalize the subject line
- Do not end the subject line with a period
- Use the imperative mood in the subject line ("Add" instead of "Added")
- Wrap the body at 80 characters
- Use the body to explain what and why vs. how
adapted from https://chris.beams.io/posts/git-commit
Have an awesome day! ☀️
odersky
force-pushed
the
add-cstate
branch
4 times, most recently
from
24574b8 to
10255b0
Compare
|
test performance please |
|
performance test scheduled: 1 job(s) in queue, 1 running. |
|
performance test failed: Please check http://lamppc37.epfl.ch:8000/pull-9372-07-19-13.50.out for more information |
|
test performance please |
|
performance test scheduled: 1 job(s) in queue, 0 running. |
|
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/9372/ to see the changes. Benchmarks is based on merging with master (21a90aa) |
|
In tried to run performance tests locally on just compiling typer/*.scala. Once all optimizations are in, I got maybe a 1% improvement. Hard to say with the noise. This could mean two things:
To test these hypotheses, c640a35 contains only the bad and not the good. I.e. we pass periods as a separate parameter but then use the context field anyway. It looks like this gives a 4 to 5 % slowdown. In any case, it seems the cost is too high to justify the gains. So I am closing this PR and will use some ideas coming out of this experiment later. |
|
I noticed the last PR was slightly than just keeping the bad. Testing again with a more precise setup. |
|
test performance please |
|
performance test scheduled: 1 job(s) in queue, 0 running. |
|
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/9372/ to see the changes. Benchmarks is based on merging with master (b7b03b3) |
Also:
- Change functions from Context to context functions. In particular:
- `newSymbol`, `requiredSymbol` etc, now are available
from Symbols, no `ctx.` prefix needed
- All reporing methods are available from `report` object.
- Add atPhase, atPhaseNoLater, addPhaseNoEarlier and have them replace
most uss of `withPhase`...
- Add inMode, withMode, withoutMode utility wrappers
- Move error messages directly into reporting: this avoids
an annoying import
- Convert old style implicit parameters to `(using Context)`
- Reorganize TyperState.test: Instead of overwriting fields of TyperState, keep test
contexts in an explicit stack, so that they can be re-used. This is simpler and
since there is more decoupling between tests. Usage is now
`Contexts.explore(...)` instead of `ctx.test(...)`.
Based on #9343. #9343 is a refactoring that improves architectural consistency. By contrast this PR is only for performance. If performance does not noticeably improve once it's done, it's not worth keeping it.