salsa-rs / salsa

A generic framework for on-demand, incrementalized computation. Inspired by adapton, glimmer, and rustc's query system.
https://salsa-rs.netlify.app/
Apache License 2.0
2.13k stars 152 forks source link

Reduce cloning of sets in `ActiveQuery` and `QueryRevisions` #567

Closed MichaReiser closed 2 months ago

MichaReiser commented 2 months ago

A small performance improvement to avoid cloning hash sets where we can consume the value instead.

netlify[bot] commented 2 months ago

Deploy Preview for salsa-rs canceled.

Name Link
Latest commit 74a4de43dc5b41a23114f889b91f3f730269c369
Latest deploy log https://app.netlify.com/sites/salsa-rs/deploys/66c6e90937271f0008dbd2be
codspeed-hq[bot] commented 2 months ago

CodSpeed Performance Report

Merging #567 will not alter performance

Comparing MichaReiser:reduce-set-cloning (74a4de4) with master (f608ff8)

Summary

✅ 8 untouched benchmarks

MichaReiser commented 2 months ago

Okay, that's not what I expected. I do see a 1-2% perf improvement in local hyperfine benchmarks

MichaReiser commented 2 months ago

Okay, that's not what I expected. I do see a 1-2% perf improvement in local hyperfine benchmarks

Using IndexSet::into_boxed_slice seems to really hurt performance (at least in codspeed). I reverted that change.

MichaReiser commented 2 months ago

I see about a 1% performance improvement on the Ruff benchmarks (second last benchmark). I hoped for more, but that's how it is with perf ;)

It seems that codspeed struggles a bit with matching the salsa frames. I suspect it doesn't help that too many frames have the same name :laughing: