Skip to content

[parallel-queries] Remove the attribute cache from the CrateStore #50508

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
michaelwoerister opened this issue May 7, 2018 · 1 comment · Fixed by #50528
Closed

[parallel-queries] Remove the attribute cache from the CrateStore #50508

michaelwoerister opened this issue May 7, 2018 · 1 comment · Fixed by #50528
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. WG-compiler-performance Working group: Compiler Performance

Comments

@michaelwoerister
Copy link
Member

The CStore maintains a cache of decoded item attributes which:

  • is an instance of shared mutable state that we'd like to avoid and
  • is largely unused at this point because item attributes are also cached via the item_attrs query.

The remaining instances where the CrateMetadata::get_item_attrs() is called directly should not be performance critical. To solve this issue:

  • Remove the attribute_cache field from CrateMetadata and make things compile again
  • Open a PR so that we can due a performance test in order to verify that this indeed does not regress compile times.

cc @rust-lang/wg-compiler-performance

@michaelwoerister michaelwoerister added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. C-enhancement Category: An issue proposing an enhancement or a PR with one. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. WG-compiler-performance Working group: Compiler Performance A-parallel-queries labels May 7, 2018
@whitfin
Copy link
Contributor

whitfin commented May 8, 2018

Hi @michaelwoerister! I took a shot at this here. From my testing everything appears to work that I can see. If you have any advice I'd appreciate it :)

bors added a commit that referenced this issue May 8, 2018
Remove attribute_cache from CrateMetadata

This PR will fix #50508 by removing the `attribute_cache` from the `CrateMetadata` struct. Seeing as performance was referenced in the original issue, I also cleaned up a `self.entry(node_id);` call which might have occasionally happened redundantly.

r? @michaelwoerister
bors added a commit that referenced this issue May 16, 2018
Remove attribute_cache from CrateMetadata

This PR will fix #50508 by removing the `attribute_cache` from the `CrateMetadata` struct. Seeing as performance was referenced in the original issue, I also cleaned up a `self.entry(node_id);` call which might have occasionally happened redundantly.

r? @michaelwoerister
bors added a commit that referenced this issue May 23, 2018
Remove attribute_cache from CrateMetadata

This PR will fix #50508 by removing the `attribute_cache` from the `CrateMetadata` struct. Seeing as performance was referenced in the original issue, I also cleaned up a `self.entry(node_id);` call which might have occasionally happened redundantly.

r? @michaelwoerister
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. WG-compiler-performance Working group: Compiler Performance
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants