Multithreaded2

[Pat-Lafon]

Hello! I've been occasionally interested in #180 which has been stalled for at least a year. I think I now have a little bit of time/interest to try this. I'm not super familiar with this code base nor in making code parallel so I would appreciate any feedback.

I'm starting from the same idea that #180 created except I am following the suggestion of using a runtime flag instead of a compile time feature flag to enable parallelism. Because of the non-trivial changes that have occurred on main, I have started from scratch instead of rebasing into #180... but all credit should go to @NishantJoshi00

I have done roughly the minimal amount of work to get started so I can get help addressing the three open questions as I understand them:

• TargetDirPool was used in the previous attempt. I haven't added it yet since I'm not sure about the interactions here but it should be trivial to add if it is still useful/the right approach.
• I have attempted to add the two previous cases that were identified in #180 as tests. These are still non-deterministically failing. As I see it there are maybe a couple of solutions: let the threads output in any order and adjust the numbering so they aren't numbered out of order, buffer the output such that the messages can be printed in a deterministic order, or something else.
• Getting the right granularity on where all of the locks are placed. I've started conservatively in following with previous work and can go from there.

[Pat-Lafon]

Oh, hmm, I see that there are ci issues because of rayon's dependencies having build scripts(outside of the two non-deterministically failing tests I had mentioned). I don't think there is much I can do there. I would hope this isn't an issue since rayon is a very popular and well-maintained but I understand if an exception cannot be made.

[taiki-e]

Thanks for the PR!

Oh, hmm, I see that there are ci issues because of rayon's dependencies having build scripts(outside of the two non-deterministically failing tests I had mentioned). I don't think there is much I can do there. I would hope this isn't an issue since rayon is a very popular and well-maintained but I understand if an exception cannot be made.

It is not very important whether the dependencies have build scripts or not, as they are only used as a reference when auditing dependencies. The error can be suppressed by adding the dependency name to the following list:

https://github.com/taiki-e/cargo-hack/blob/74823c8a30bb37a00b6b016ff90aeba915b88bbd/.deny.toml#L22-L25

[Pat-Lafon]

@m-esposito I rebased on main and took your suggestion for &Arc -> Arc though it seems to run afoul with clippy. Do you think it is a false positive with clippy?

The if statement still has 4 branches regardless of how I reorganize it no? I guess I don't mind but it doesn't seem like much of an improvement to me.

Do you have any suggestions for actually verifying that code is running in parallel as expected and isn't blocked in some way? I don't have much context for this code but I would imagine that is the main blocker

[m-esposito]

Hm, re the &Arc, it seems like your original method is correct. https://stackoverflow.com/questions/55576425/why-does-clippy-suggests-passing-an-arc-as-a-reference Sorry for the nit noise.

About the benchmarking, I did some preliminary testing and it seems like it's not running in parallel. Cargo invocations can't run in parallel without splitting away the target directories (noticeably when you save in an IDE w/ rust-analyzer, with check enabled on every save, then you try cargo run - it blocks until rust-analyzer's cargo invocation finishes), so TargetDirPool is necessary.