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.

Sign up for GitHub

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

Jump to bottom

Remove mixin traits of Context #9343

Merged
merged 10 commits into from
Jul 20, 2020
Merged

Remove mixin traits of Context #9343

merged 10 commits into from
Jul 20, 2020

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Jul 10, 2020 •
edited
Loading

In particular:

  • newSymbol, requiredSymbol etc, now are available
    from Symbols, no ctx. prefix needed
  • All reporing methods are available from report object.

It's clearer to make them regular objects. Also:

  • Change functions from Context to context functions.
  • 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(...).

@odersky odersky force-pushed the change-using branch 5 times, most recently from bce1767 to 37e4b27 Compare July 14, 2020 08:41
@odersky
Copy link
Contributor Author

odersky commented Jul 14, 2020

test performance please

@dottybot
Copy link
Member

performance test scheduled: 1 job(s) in queue, 0 running.

@dottybot
Copy link
Member

performance test failed:

Please check http://lamppc37.epfl.ch:8000/pull-9343-07-14-22.32.out for more information

@odersky
Copy link
Contributor Author

odersky commented Jul 15, 2020

test performance please

@dottybot
Copy link
Member

performance test scheduled: 1 job(s) in queue, 0 running.

@liufengyun
Copy link
Contributor

From the logs:

[error] -- [E008] Not Found Error: /data/workspace/bench/dotty/bench/src/main/scala/Benchmarks.scala:105:14 
[error] 105 |          ctx.error(ex.getMessage) // signals that we should fail compilation.
[error]     |          ^^^^^^^^^
[error]     |value error is not a member of dotty.tools.dotc.core.Contexts.Context - did you mean ctx.period?
[error] one error found

@dottybot
Copy link
Member

performance test failed:

Please check http://lamppc37.epfl.ch:8000/pull-9343-07-15-10.12.out for more information

@odersky
Copy link
Contributor Author

odersky commented Jul 15, 2020

test performance please

@dottybot
Copy link
Member

performance test scheduled: 1 job(s) in queue, 0 running.

@dottybot
Copy link
Member

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/9343/ to see the changes.

Benchmarks is based on merging with master (001817c)

@odersky odersky mentioned this pull request Jul 16, 2020
Closed
@odersky
Copy link
Contributor Author

odersky commented Jul 16, 2020

We seem to have a small but significant improvement of performance in stdlib compilation. The rest is more or less the same. The compiler itself is a little bit slower, which is to be expected since there are now many more implicit searches than before.

@odersky
Copy link
Contributor Author

odersky commented Jul 16, 2020

Squashed to one commit to allow for easier rebasing

@odersky odersky force-pushed the change-using branch 2 times, most recently from c09830d to 3524a5b Compare July 17, 2020 09:13
@nicolasstucki nicolasstucki mentioned this pull request Jul 17, 2020
Merged
@odersky odersky force-pushed the change-using branch 3 times, most recently from 95b56a8 to b81cc98 Compare July 20, 2020 13:12
odersky added 5 commits July 20, 2020 15:26
@odersky
Move importInfo to Context store
bd339f5 
Stats show that importInfo is updated about 10x less that Store.
E.g. compiling Dotty:

Total context creations: 5.6M
Store creations:          68K
Import info creations:   6.8K

So this means we save 22.4M be removing importInfo as a field of contexts
at a price of less than 600K for the added store creations.

# Conflicts:
#	compiler/src/dotty/tools/dotc/core/Contexts.scala
@odersky
Change test logic.
3715a10 
Use a freelist of unsused contexts instead of overwriting typerstate variables.
Rename from `ctx.test` to `explore`.
@odersky
Stats recording for computing denotations
dd8a198
@odersky
Remove all mixin traits from Contexts
7769a3c 
Also:

 - Change functions from Context to context functions
 - Rename ctx.erasedTypes -> currentlyAfterErasure
      ctx.isAfterTyper -> currentlyAfterTyper
 - Add currentPhase, currentPeriod, ... utility methods
 - Replace `c.runId` by `currentRunId(using c)`. This can be
   changed back again if we end up not splitting period info
   from contexts
 - Add inMode, withMode, withoutMode utility wrappers
 - Move error messages directly into reporting: this avoids
   an annoying import
@odersky
Simplify context handling
f2d8842 
 - Use only wildcard imports from Contexts
 - Use `(using Context)` throughout
odersky added 5 commits July 20, 2020 15:27
@odersky
Fix rebase breakage
fc803f2
@odersky
Move runId, phaseId back to context
1679663
@odersky
Move phase back to Context
2ccebc5
@odersky
Move currentPeriod back to Context
0e42cd8
@odersky
Remove all mixin traits from Contexts. In particular:
0e0f686 
 - `newSymbol`, `requiredSymbol` etc, now are available
   from Symbols, no `ctx.` prefix needed
 - All reporing methods are available from `report` object.

Also:

 - Change functions from Context to context functions.
 - 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(...)`.

# Conflicts:
#	compiler/src/dotty/tools/dotc/core/Definitions.scala

# Conflicts:
#	compiler/src/dotty/tools/dotc/Run.scala
#	compiler/src/dotty/tools/dotc/core/Annotations.scala
#	compiler/src/dotty/tools/dotc/core/Contexts.scala
#	compiler/src/dotty/tools/dotc/core/Definitions.scala
#	compiler/src/dotty/tools/dotc/core/Phases.scala
#	compiler/src/dotty/tools/dotc/core/TyperState.scala
#	compiler/src/dotty/tools/dotc/core/Types.scala
#	compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala
#	compiler/src/dotty/tools/dotc/transform/ExplicitOuter.scala
#	compiler/src/dotty/tools/dotc/transform/TreeChecker.scala
#	compiler/src/dotty/tools/dotc/typer/Namer.scala
@odersky
Copy link
Contributor Author

odersky commented Jul 20, 2020

test performance please

@dottybot
Copy link
Member

performance test scheduled: 1 job(s) in queue, 0 running.

@dottybot
Copy link
Member

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/9343/ to see the changes.

Benchmarks is based on merging with master (883198c)

@odersky odersky merged commit b2dc101 into scala:master Jul 20, 2020
@odersky odersky deleted the change-using branch July 20, 2020 17:08
@sjrd sjrd mentioned this pull request Jul 22, 2020
Merged
@odersky odersky mentioned this pull request Jul 23, 2020
Closed
smarter added a commit to dotty-staging/dotty that referenced this pull request Jul 23, 2020
@smarter
Fix sbt scripted tests after refactor in scala#9343
0d3c674
anatoliykmetyuk added a commit that referenced this pull request Jul 23, 2020
@anatoliykmetyuk
Merge pull request #9422 from dotty-staging/fix-sbt-ctx
eb3b0ac 
Fix sbt scripted tests after refactor in #9343
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@odersky @dottybot @liufengyun