Delay snapshotting of desugaring environment #3585
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…o it when necessary!
mtzguido
force-pushed
the
nik_delay_dsenv_snapshot
branch
from
7132657
to
5a2ee19
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Snapshotting a desugaring environment is linear in the number of definitions in scope
We currently snapshot the environment before processing any declaration, which leads to quadratic behavior for desugaring modules with a large number of definitions. Thanks @LukeXuan for noting this!
The snapshotting is needed to properly handle definitions marked with an "expect_failure" attribute. This PR snapshots the environment only when processing such a decorated definition, rather than unconditionally snapshotting it. This avoids the quadratic behavior in the common case of definitions that are not decorated with expect_failure
A better long-term solution would be to move to the use of persistent maps in desugaring, making such snapshots constant time.
(Note, there is still some quadratic behavior left in the desugaring phase, as name resolutions involve a linear scan---working on fixing that in a separate PR)