Experiment: Per ingredient sync table

[Veykril]

Both memo table and sync tables increase the size of memos by quite a bit, so this replaces the per field locks with a per ingredient lock. I believe given that the lock is held fairly shortly we ought to be fine with this coarser grained locking (lock per ingredient to claim a sync). Alternatively we could make use of a DashMap as a tradeoff.

[netlify]

✅ Deploy Preview for salsa-rs canceled.

Name	Link
🔨 Latest commit	e2c4b38
🔍 Latest deploy log	https://app.netlify.com/sites/salsa-rs/deploys/67990a55b7e7c50008c04e95

[codspeed-hq]

CodSpeed Performance Report

Merging #650 will not alter performance

_{Comparing Veykril:veykril/push-rxpmtpukknru (e2c4b38) with master (8c49bb0)}

Summary

✅ 9 untouched benchmarks

[davidbarsky]

I rebased this change atop of a few others (#649 and Chayim's enum changes), and this change reduced rust-analyzer's analysis-stats memory usage by 240 megabytes: overall memory usage is down to 3360mb, down from an initial 8 gigabytes!

However, I'm unsure of of the impact of this when it comes to parallelism, so I don't feel comfortable stamping this PR.

[ChayimFriedman2]

@davidbarsky As an alternative I have my branch (unpublished), that shrinks SyncState from 16 to 2 bytes, without hurting parallelism. I don't remember how much it saves compared to this branch, but I can check.

[Veykril]

I think putting up your changes for discussion would be good in general (as mine do change semantics wrt to parallelism)

[davidbarsky]

Lukas, I assume you're replying to Chayim, but here are the branches I have right now:

• rust-analyzer on new Salsa: rust-lang/rust-analyzer@master...davidbarsky:rust-analyzer:davidbarsky/port-rust-analyzer-to-new-salsa
• All the Salsa changes used by rust-analyzer on new Salsa: master...davidbarsky:salsa:chayim-david-lukas/salsa-perf-improvements

Notably, these are without Chayim's enum changes being used in rust-analyzer, which I'll port over now.

[davidbarsky]

@davidbarsky As an alternative I have my branch (unpublished), that shrinks SyncState from 16 to 2 bytes, without hurting parallelism. I don't remember how much it saves compared to this branch, but I can check.

Feel free to check, but putting up the branch (even in a draft state!) would be great.

[ChayimFriedman2]

So I checked and on my machine, Lukas's changes save 255mb and my changes save 133mb. I'm working on finalizing and publishing my branch.

[davidbarsky]

For posterity/context for tomorrow's meeting, I believe Chayim's changes are on this branch: master...ChayimFriedman2:salsa:sync-v2.

[Veykril]

So, it would be nice for rust-analyzer to have this due to the memory decrease, I am not sure about the concurrency impact this has though. I would expect it to marginally be worse in that there is a coarser grained lock involved (within the ingredient), but that should only be an issue when there is high contention for the same ingredient so I believe this ought to be fine. (also adjusted the PR description)

[Veykril]

I'll draft this for now, I think we can consider this once r-a is properly moved over and more parallelized to see if this has an effect or not