Skip to content
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

perf: Store view downcaster in function ingredients directly #720

Merged
merged 4 commits into from
Mar 6, 2025

Conversation

Veykril
Copy link
Member

@Veykril Veykril commented Feb 23, 2025

Allows us to skip iterating over all registered view casters whenever we need to cast

Copy link

netlify bot commented Feb 23, 2025

Deploy Preview for salsa-rs canceled.

Name Link
🔨 Latest commit 1772263
🔍 Latest deploy log https://app.netlify.com/sites/salsa-rs/deploys/67c9769e10fb8e0008de4063

Copy link

codspeed-hq bot commented Feb 23, 2025

CodSpeed Performance Report

Merging #720 will improve performances by 7.02%

Comparing Veykril:veykril/push-ktqyuntnwvxn (1772263) with master (ceb9b08)

Summary

⚡ 1 improvements
✅ 10 untouched benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
accumulator 3.4 ms 3.2 ms +7.02%

@Veykril Veykril force-pushed the veykril/push-ktqyuntnwvxn branch from f23162c to 8f455b0 Compare February 23, 2025 11:41
@Veykril Veykril force-pushed the veykril/push-ktqyuntnwvxn branch from 8f455b0 to d91e86b Compare February 23, 2025 12:01
@Veykril
Copy link
Member Author

Veykril commented Feb 23, 2025

The benchmark results are a bit confusing. I think getting rid of the arc-swap stuff might also help making the benches more reliable

@Veykril Veykril force-pushed the veykril/push-ktqyuntnwvxn branch 4 times, most recently from 80c41b5 to 169a194 Compare February 24, 2025 14:33
@Veykril
Copy link
Member Author

Veykril commented Feb 24, 2025

The benchmark results are a bit confusing. I think getting rid of the arc-swap stuff might also help making the benches more reliable

Seems to be more stable with the alloc change now (average green now)

@Veykril Veykril force-pushed the veykril/push-ktqyuntnwvxn branch from 169a194 to 18d4f9c Compare February 25, 2025 15:26
@Veykril Veykril force-pushed the veykril/push-ktqyuntnwvxn branch 2 times, most recently from 2da5ca3 to dd26a21 Compare February 26, 2025 06:42
@Veykril
Copy link
Member Author

Veykril commented Feb 26, 2025

Decent improvement on the accumulator bench

@Veykril Veykril changed the title Store view downcaster in function ingredients directly [perf] Store view downcaster in function ingredients directly Mar 5, 2025
@nikomatsakis nikomatsakis changed the title [perf] Store view downcaster in function ingredients directly perf: Store view downcaster in function ingredients directly Mar 5, 2025
Copy link
Member

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. I don't see any issues.

@Veykril Veykril force-pushed the veykril/push-ktqyuntnwvxn branch from dd26a21 to 00f8757 Compare March 6, 2025 10:17
@Veykril Veykril force-pushed the veykril/push-ktqyuntnwvxn branch from 00f8757 to 1772263 Compare March 6, 2025 10:19
@Veykril Veykril enabled auto-merge March 6, 2025 10:19
@Veykril Veykril added this pull request to the merge queue Mar 6, 2025
Merged via the queue into salsa-rs:master with commit 9d2a978 Mar 6, 2025
11 checks passed
@Veykril Veykril deleted the veykril/push-ktqyuntnwvxn branch March 6, 2025 11:00
@github-actions github-actions bot mentioned this pull request Mar 6, 2025
carljm added a commit to carljm/salsa that referenced this pull request Mar 8, 2025
* master:
  internal: use `portable-atomic` in `IngredientCache` to compile on `powerpc-unknown-linux-gnu` (salsa-rs#749)
  perf: Store view downcaster in function ingredients directly (salsa-rs#720)
  perf: Some small perf things (salsa-rs#744)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants