Some further optimizations #3223
Conversation
Hash-consing used to use `equals`, except in some cases, where it used `eq`. This was inconsistent, and `equals` is also needlessly expensive.
| @@ -606,6 +606,7 @@ object Contexts { | |||
| /** A table for hash consing unique types */ | |||
| private[core] val uniques = new util.HashSet[Type](Config.initialUniquesCapacity) { | |||
| override def hash(x: Type): Int = x.hash | |||
| override def isEqual(x: Type, y: Type) = x.eql(y) | |||
There was a problem hiding this comment.
Shouldn't the override also be used in the other subclasses of HashSet ? That is, AppliedUniques and NamedTypeUniques
There was a problem hiding this comment.
No, equals is never called on those sets. Everything is based on eq for them.
| @@ -381,25 +377,31 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] => | |||
|
|
|||
| def isPureExpr(tree: Tree)(implicit ctx: Context) = exprPurity(tree) == Pure | |||
| def isIdempotentExpr(tree: Tree)(implicit ctx: Context) = exprPurity(tree) >= Idempotent | |||
| def isReadOnlyExpr(tree: Tree)(implicit ctx: Context) = exprPurity(tree) >= ReadOnly | |||
There was a problem hiding this comment.
This commit would benefit from some testcases. I suggest adding some tests like https://github.com/lampepfl/dotty/blob/1e0863708e48339759f73f38464a410df850df15/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala#L168-L189 which check that two methods end up with the same bytecode generated.
|
The second commit results in additional incorrect warnings: [warn] -- Warning: /tmp/3/compiler/src/dotty/tools/dotc/core/tasty/NameBuffer.scala:47:4
[warn] 47 | op
[warn] | ^^
[warn] | a read-only expression does nothing in statement positionI suggest breaking off the second commit in a separate PR since it's not an optimization, and we can get the optimization in now. |
odersky
force-pushed
the
try-optimize-3
branch
from
411e040 to
93f0ff3
Compare
|
@smarter Yes, good idea. |
|
test performance please |
|
performance test scheduled: 1 job(s) in queue, 0 running. |
|
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/3223/ to see the changes. Benchmarks is based on merging with master (b570a62) |
|
As before, these data points have to be looked at carefully because they're not including today's merged PRs. @liufengyun are the PR data points generated after every PR is merged or daily? |
|
@smarter The daily points are generated daily at mid-night. |
|
Would it be possible to generate them everytime a PR is merged instead? |
|
Yes, we can change it to run immediately after merge. I'll make it the next TODO in bench. |
No description provided.