Skip to content

perf: Allow toggling events off statically - #1287

Open
ChayimFriedman2 wants to merge 1 commit into
salsa-rs:masterfrom
ChayimFriedman2:no-event
Open

perf: Allow toggling events off statically#1287
ChayimFriedman2 wants to merge 1 commit into
salsa-rs:masterfrom
ChayimFriedman2:no-event

Conversation

@ChayimFriedman2

Copy link
Copy Markdown
Contributor

They are typically only used for tests, and removing them statically removes a load+branch and sometimes more (the callback can pessimize compiler optimizations).

This will fail tests as they need this. I'll fix this, but this will prevent seeing the benchmark impact, so I'm first opening the PR without.

@netlify

netlify Bot commented Aug 12, 2026

Copy link
Copy Markdown

Deploy Preview for salsa-rs canceled.

Name Link
🔨 Latest commit cd235b9
🔍 Latest deploy log https://app.netlify.com/projects/salsa-rs/deploys/6a7cb1b00fa6ea0009069ef6

@ChayimFriedman2

Copy link
Copy Markdown
Contributor Author

Seems insignificant. But maybe still worth it given how easy this is? Not sure.

They are typically only used for tests, and removing them statically removes a load+branch and sometimes more (the callback can pessimize compiler optimizations).
@MichaReiser

Copy link
Copy Markdown
Contributor

I suspect that it's not significant, but the best way to tell is to run some benchmarks on ty/rust-analyzer to see if there's a meaningful difference. If not, I'm inclined to keep as is.

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.

2 participants