Skip to content

stage2: store Cache files in a map data structure #7393

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

Closed
wants to merge 1 commit into from

Conversation

andrewrk
Copy link
Member

@andrewrk andrewrk commented Dec 11, 2020

Cache now stores files as an StringArrayListHashMap instead of an
ArrayList. The key is the resolved file path, to avoid adding the same
file multiple times into the cache system when it is depended on
multiple times. This avoids re-hashing file contents that have already
been inserted into the cache.

This was an attempt to fix #7308, but it is solving a different problem.
I do think it is worth considering this improvement regardless. On the
other hand, it might be wasted CPU cycles / code bloat since the layer of code above
this one probably wants to do its own de-duplication as well.

This implementation would be much nicer with #7391.

Cache now stores files as an StringArrayListHashMap instead of an
ArrayList. The key is the resolved file path, to avoid adding the same
file multiple times into the cache system when it is depended on
multiple times. This avoids re-hashing file contents that have already
been inserted into the cache.

This was an attempt to fix #7308, but it is solving a different problem.
I do think it is worth considering this improvement regardless. On the
other hand, it might be duplicated effort since the layer of code above
this one probably wants to do its own de-duplication as well.

This implementation would be much nicer with #7391.
@andrewrk andrewrk added proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. enhancement Solving this issue will likely involve adding new logic or components to the codebase. frontend Tokenization, parsing, AstGen, Sema, and Liveness. labels Dec 11, 2020
@andrewrk
Copy link
Member Author

Well tests are failing and I wasn't even sure if this was an improvement, so I'm going to throw this patch in the garbo

@andrewrk andrewrk closed this Dec 11, 2020
@Vexu Vexu deleted the stage2-cache-map branch December 11, 2020 09:27
@Vexu Vexu restored the stage2-cache-map branch December 11, 2020 09:28
@andrewrk andrewrk deleted the stage2-cache-map branch December 29, 2021 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Solving this issue will likely involve adding new logic or components to the codebase. frontend Tokenization, parsing, AstGen, Sema, and Liveness. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

adding the same C source file redundantly to the same compilation results in a deadlock
1 participant