Force recompilation if imported module has changed #3703
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
radeusgd
approved these changes
Sep 14, 2022
There was a problem hiding this comment.
Looks good to me.
As we don't have tests, we need to ensure manually that it works as expected:
- Did you try the scenario from the bug report to verify that now it behaves correctly?
- Can you somehow verify if after the changes there are not "too" many invalidations? (I guess just running a program twice, one with cleared caches and one cached would be ok to see that the second one is faster - very imperfect test but better than nothing)
Yeah, I did all of that. |
radeusgd
approved these changes
Sep 14, 2022
hubertp
force-pushed
the
wip/hubert/cache-invalidation-182953733
branch
from
fead139 to
757eec9
Compare
JaroslavTulach
approved these changes
Sep 14, 2022
engine/runtime/src/test/resources/Test_Caching_Invalidation/src/Main.enso
Outdated
Show resolved
Hide resolved
IR cache never really took into account a situation when a binding from the imported module has changed. In other words, it would continue to happily use the serialized metadata without noticing that it changed. This change forces cache invalidation when any of the imported modules was invalidated (or rather not loaded from cache). Sadly, it doesn't appear that we have appropriate infrastructure to write such scenarios so I'm delaying that bit for now.
Using _x, where x is the change step, to simulate modifications to files and thus trigger cache invalidation that should propagate to upstream modules.
hubertp
force-pushed
the
wip/hubert/cache-invalidation-182953733
branch
from
fed45c3 to
9293020
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.
Pull Request Description
IR cache never really took into account a situation when a binding from the imported module has changed. In other words, it would continue to happily use the serialized metadata without noticing that it changed.
This change forces cache invalidation when any of the imported modules was invalidated (or rather not loaded from cache).
Important Notes
Added simple test infrastructure that simulates file modifications that would trigger the initial cache invalidation.
If they succeed, cache invalidation is propagated thus causing an error.
Checklist
Please include the following checklist in your PR:
Scala,
Java,
and
Rust
style guides.
./run ide buildand./run ide watch.