-
Notifications
You must be signed in to change notification settings - Fork 1.1k
More efficient garbage collection of type variables #4447
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
More efficient garbage collection of type variables #4447
Conversation
test performance please |
performance test scheduled: 1 job(s) in queue, 1 running. |
Two tests fail due to what looks like underlying issues to me, I'd appreciate some guidance here:
cannot merge Constraint(
uninstVars = A;
constrained types = [A, B](elems: (A, B)*): Map[A, B]
bounds =
A >: Char
B := Boolean
ordering =
) with Constraint(
uninstVars = Boolean;
constrained types = [A, B](elems: (A, B)*): Map[A, B]
bounds =
A := String
B <: Boolean
ordering =
), mergeEntries( >: Char, String)
at dotty.tools.dotc.core.OrderingConstraint.mergeEntries$1(OrderingConstraint.scala:532) The fix in #4276 is not enough anymore since we now try to merge a TypeBounds and a non-TypeBounds. The underlying issue is that we try to |
Performance test finished successfully: Visit http://dotty-bench.epfl.ch/4447/ to see the changes. Benchmarks is based on merging with master (d0f7846) |
For tests/pos/i1590.scala can we simply not eliminate constraints if they contain wildcard types? Or else do the approximation on elimination as well? We do get these constraints because we approximate type variables in expected implicit types and values with wildcard types in order to get good cache hit rates for implicit eligibility caches. Fir tests/neg/i4272.scala I agree, we should not commit, but simply flush the reporter. |
When narrowing a constraint, we check if its bounds are now equal to remove it from the constraint set. This allows the corresponding type variable to be instantiated in TyperState#gc instead of accumulating uninstantiated type variables that we traverse again and again in Inferencing#interpolateTypeVars. This reduces the running time of the added testcase (copied from scala/scala#6580) from ~6 seconds to ~4 seconds. If we fully uncomment the testcase (and increase the JVM stack size), the running time goes from > 300 seconds to ~24 seconds. This is not as good as what scala/scala#6580 achieves but is already much more reasonable that what we did before.
4797b8a
to
eeb5965
Compare
After further discussion, here's the fixes we agreed on and that are now implemented:
|
When narrowing a constraint, we check if its bounds are now equal to
remove it from the constraint set. This allows the corresponding type
variable to be instantiated in TyperState#gc instead of accumulating
uninstantiated type variables that we traverse again and again in
Inferencing#interpolateTypeVars.
This reduces the running time of the added testcase (copied from
scala/scala#6580) from ~6 seconds to ~4 seconds.
If we fully uncomment the testcase (and increase the JVM stack size),
the running time goes from > 300 seconds to ~24 seconds. This is not as
good as what scala/scala#6580 achieves but is
already much more reasonable that what we did before.