Use Eio for parallelizing some analysis commands
This introduces OCaml multicore (Eio) to the analysis bin, and leverages it for 2 of the editor tooling commands:
rename- Find all references
Benching this on my local machine, in a repo of ~350k lines of ReScript and ~1800 files, both commands end up about 2.5x faster.
rescript
npm i https://pkg.pr.new/rescript-lang/rescript@7840
@rescript/darwin-arm64
npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-arm64@7840
@rescript/darwin-x64
npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-x64@7840
@rescript/linux-arm64
npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-arm64@7840
@rescript/linux-x64
npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-x64@7840
@rescript/runtime
npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/runtime@7840
@rescript/win32-x64
npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/win32-x64@7840
commit: 4d74fcc
Could you address Copilot's comments? Other than that, looks good to me! 👍
I thought it was all straightforward until I saw the locks. Now the question: why did you add locks and how do you know that we don't have deadlocks (locking too much) or data races (some locks are still missing)
Worth spending some time of this ahead of time before getting into bugs that are very hard to reproduce.
There were races in accessing/writing to the cmt cache, that caused random failures of the command. I'll do another round of reasoning about the locks etc.
I thought it was all straightforward until I saw the locks. Now the question: why did you add locks and how do you know that we don't have deadlocks (locking too much) or data races (some locks are still missing) Worth spending some time of this ahead of time before getting into bugs that are very hard to reproduce.
There were races in accessing/writing to the cmt cache, that caused random failures of the command. I'll do another round of reasoning about the locks etc.
How about making sure that transitively no mutable global state is accessed? Is that too hard to establish?
How about making sure that transitively no mutable global state is accessed? Is that too hard to establish?
No that's probably a great idea. I'll give it a shot! It's great if we figure these things out now, because this type of Eio thing will hopefully be the foundation of quite a few things going forward.
How about making sure that transitively no mutable global state is accessed? Is that too hard to establish?
No that's probably a great idea. I'll give it a shot! It's great if we figure these things out now, because this type of Eio thing will hopefully be the foundation of quite a few things going forward.
So no need for locks and things would be much safer. Though: one needs to be careful not to introduce a slowdown.
@cristianoc https://github.com/rescript-lang/rescript/pull/7840/commits/43527087e9794e76b0a85a607ea61ed10bb68173 removes mutexes in favor of domain local caches. No meaningful regression in perf from doing this. But can't help to wonder if there's a better way.
We'll of course continue improving on this as we integrate more features with the same functionality. This first version needs to be safe and clear/understandable though of course.