Replace unnecessary Arc by Box

[Veykril]

QueryEdges were Arced before, but we only ever clone them for LRU evictions (and there the original one will drop right after eviction), this is a rare occurence. Additionally to save on constantly allocating an empty Arc in the case where no deps exist, we needed a lazy_static. Instead, we Box the slice now, dropping the need for ref-counting as well as the lazy_static as an empty slice never allocates in a Box.

Another improvement we should make is use a ThinBox once that's a thing, we don't really make use of the length directly and it would shave off 4/8 bytes for Memo/QueryRevisions.

[netlify]

✅ Deploy Preview for salsa-rs canceled.

Name	Link
🔨 Latest commit	38207e4
🔍 Latest deploy log	https://app.netlify.com/sites/salsa-rs/deploys/67694e2784fbad0008d31d9d

[codspeed-hq]

CodSpeed Performance Report

Merging #635 will not alter performance

_{Comparing Veykril:veykril/push-vktlqysrmlmv (38207e4) with master (0ac5c1c)}

Summary

✅ 9 untouched benchmarks

[Veykril]

---- parallel_map::execute_cancellation stdout ----
[tests/parallel/signal.rs:11:9] format!("signal({})", stage) = "signal(1)"
[tests/parallel/signal.rs:11:9] format!("signal({})", stage) = "signal(2)"
thread 'parallel_map::execute_cancellation' panicked at tests/parallel/parallel_map.rs:91:10:
called `Result::unwrap_err()` on an `Ok` value: [2, 3, 4, 5, 6, 7, 8, 9, 10, 11]

seems spurious to me as I can't reproduce it

[MichaReiser]

---- parallel_map::execute_cancellation stdout ----
[tests/parallel/signal.rs:11:9] format!("signal({})", stage) = "signal(1)"
[tests/parallel/signal.rs:11:9] format!("signal({})", stage) = "signal(2)"
thread 'parallel_map::execute_cancellation' panicked at tests/parallel/parallel_map.rs:91:10:
called `Result::unwrap_err()` on an `Ok` value: [2, 3, 4, 5, 6, 7, 8, 9, 10, 11]

seems spurious to me as I can't reproduce it

I don't remember the test being spurious before... But @davidbarsky might know more

[Veykril]

Uuh, I cancelled the first CI run as I pushed twice and apparently that just skips the merge queue? 🤨

[Veykril]

---- parallel_map::execute_cancellation stdout ----
[tests/parallel/signal.rs:11:9] format!("signal({})", stage) = "signal(1)"
[tests/parallel/signal.rs:11:9] format!("signal({})", stage) = "signal(2)"
thread 'parallel_map::execute_cancellation' panicked at tests/parallel/parallel_map.rs:91:10:
called `Result::unwrap_err()` on an `Ok` value: [2, 3, 4, 5, 6, 7, 8, 9, 10, 11]

seems spurious to me as I can't reproduce it

I don't remember the test being spurious before... But @davidbarsky might know more

Not the first time it seems, it even failed on master before https://github.com/salsa-rs/salsa/actions/runs/12397086201/job/34606629011

[davidbarsky]

I don't remember the test being spurious before... But @davidbarsky might know more

This was a recent development, but it seems consistently spurious on CI. I'm unable to reproduce this issue locally, however, which feels a bit concerning to me.

[Veykril]

I can reproduce it by making the main thread sleep for a bit after spawning the other thread it seems

[davidbarsky]

I can reproduce it by making the main thread sleep for a bit after spawning the other thread it seems

Ah, yeah, this is sufficient for me to reproduce:

diff --git a/tests/parallel/parallel_map.rs b/tests/parallel/parallel_map.rs
index 80e4ebeafe..278bbfedd8 100644
--- a/tests/parallel/parallel_map.rs
+++ b/tests/parallel/parallel_map.rs
@@ -79,6 +79,7 @@
         let db = db.clone();
         move || a1(&db, input)
     });
+    std::thread::sleep(std::time::Duration::from_millis(100));

     let counts = (2..=20).collect::<Vec<u32>>();

[Veykril]

Ah I can see the problem I think, by the time the signal passes we will no longer do any cancellable work in the a1 function, so we never end up cancelling (note how the test in parallel_cancellation differs by calling a query after the wait_for call)