Skip to content

perf: Collect module symbols in parallel #19711

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

Conversation

ChayimFriedman2
Copy link
Contributor

This saves 2 seconds when cache priming omicron on my computer.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 28, 2025
@Veykril
Copy link
Member

Veykril commented Apr 29, 2025

That suprises me to be honest as that sounds like we aren't utilizing a lot of threads in cache priming?

@ChayimFriedman2
Copy link
Contributor Author

ChayimFriedman2 commented Apr 29, 2025

First, I have a lot of cores in my machine (32 to be precise), so that's maybe a factor (but for people with less cores this probably won't be worse; at most, not better). But also, we populate symbols for local crates, and at that point, the dependency graph narrows and we indeed don't have a lot of parallelism.

@Veykril
Copy link
Member

Veykril commented Apr 29, 2025

Well my thinking is that the module symbols query should not depend on other module symbols, and at this stage we already have populated all def maps. So the graph narrowing should not matter that much. Not opposed to this here though, just something I am a bit confused by.

@Veykril Veykril added this pull request to the merge queue Apr 29, 2025
@ChayimFriedman2 ChayimFriedman2 removed this pull request from the merge queue due to a manual request Apr 29, 2025
@ChayimFriedman2
Copy link
Contributor Author

I realized we don't put cache priming in rayon threads, so this may cause conflicts. I will fix this.

@Veykril
Copy link
Member

Veykril commented May 5, 2025

Closing in favor of #19721

@Veykril Veykril closed this May 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants