-
Notifications
You must be signed in to change notification settings - Fork 13.2k
Instead of maintaining cache id etc, reset the "working" exports map cache. #48579
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
Conversation
|
@amcasey can you check if this is helpful? |
|
My notes from the time: #45283 |
6ae2684 to
b83b4ad
Compare
| BuilderState.updateSignaturesFromCache(state, state.currentAffectedFilesSignatures!); | ||
| state.currentAffectedFilesSignatures!.clear(); | ||
| BuilderState.updateExportedFilesMapFromCache(state, state.currentAffectedFilesExportedModulesMap); | ||
| state.currentAffectedFilesExportedModulesMap?.clear(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this got missed when we first introduced currentAffectedFilesExportedModulesMap.. Look how we clear currentAffectedFilesSignatures just above..
This reverts commit 0f6e6ef.
…to final exports map
b83b4ad to
3ac9226
Compare
|
As I mentioned offline, I think we need to confirm this doesn't cause a real-world regression. I'm trying to get set up to validate that now. |
|
Edit: I should probably try a project mode build, just in case. |
amcasey
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perf looks comparable on the project that motivated the change.
…cache. (microsoft#48579) * Revert "Avoid no-op export map updates (microsoft#45238)" This reverts commit 0f6e6ef. * Need to reset currentAffectedFilesExportedModulesMap after commiting to final exports map
Is this better alternative to #45238