Open AndrasKovacs opened 1 month ago
@plt-amy I'd like to benchmark this on 1lab, what's the best way? I asked this from you before but I don't find where I did.
More or less as expected, we OOM on 1lab.HIT.Truncation
as described in #7231.
To summarize the discussion there/our findings from the AIM, the issue is essentially that keeping around the telescopes for solved (primarily instance) metas in macros leads to quadratic duplication of intermediate results:
foo : {{ _ : Class }} → TC ⊤
test : TC ⊤
test = do
x ← something -- imagine it produces a very big result
foo {{ ?inst ... x }}
After evaluating something
and bindTC
, we're looking at foo {{ ?inst ... [very big result ] }}
, and this result is further duplicated in every bind that happens after something
. I think this doesn't usually matter, but if something has to traverse the term (like addContext
, for the escape check), it's explosive.
@plt-amy
freeIn
bug? TC
-produced terms are generally not instantiated, unlike ordinary definitions?AIUI the problem isn't in the generated terms, but rather in the macros themselves — the duplicated intermediate results are present in meta telescopes in suspended computations (after using e.g. blockTC), and the results of TC computations. As for the actual root cause, I can't say: the uninstantiated term is ~5× bigger (printed) than the instantiated term, but we didn't perform any investigation other than "instantiateFull fixes it" (and looking for ⦃ ... ⦄
and going "huh, that shouldn't be there").
(Of course, #7231 should be updated to normalise
the intermediate result, since otherwise you get false positives for local variables that would evaluate away; this is unrelated, but also fixes the OOM).
--save-metas
the default.Dead code elimination now does the following:
eliminateDeadCode
computes and returns collections of reachable definitions and metas. It does not instantiate anything and has no side effect. The roots for reachability are the following:buildInterface
, weinstantiateFull
the output interface if we have--no-save-metas
.Moreover,
--save-metas
is now made default. Reasons for doing so:--save-metas
, and--no-save-metas
should eventually become deprecated in favor of finer control over meta inlining.--save-metas
should be the config that's being continuously tested. While making this branch, I discovered thatmake test
actually failed with--save-metas
because of an unrelated bug inNamesIn
(which is fixed here).In short there's a significant performance win in
std-lib
and an insignificant loss incubical
. I'd consider the interface size diffs to be also insignificant.Right now, part of the small impact of
--save-metas
is that all definition bodies areinstantiateFull
-ed by elaboration inAgda.TypeChecking.Rules.Def
. So far, I think that the following metas can survive elaboration: those in function clause LHS-es, rewrite rules, display forms, module parameter sections, and some metas in definitions bodies, those that become solved later by elaborating other things in a mutual group.