Reorganize modes -safe, -fast, -faster

[aantron]

There are several issues here.

1. -fast is also apparently safe. @rleonid if -fast is faster than -safe (is it?), why is there -safe? Is -safe more correct than -fast in some case?
2. I want to write a little performance test so we can understand the real performance of the various modes.
3. I want to also write a race condition test.
4. The difference between -fast and -faster is that -faster doesn't try to take locks even if you link in BisectThread.
   • The reason for locks in either of these modes is that they increment counts. Are counts actually often used from Bisect_ppx? If we were only updating true/false flags, we could write true without locks in all cases. Maybe what we need instead of -fast and -faster is -boolean and -numeric, and if you choose -numeric, it is always correct, but you pay a price for locks.
   • If we don't want to do this, I want to look at the actual performance difference between -fast with locks and -faster. It's probably significant, but if it isn't, I would like to eliminate -faster.
   • There may also be a way to use the ocaml.ppx.context attribute, or some other method, to find out if we need to take locks. This would eliminate the brittleness of BisectThread, but also take per-module control over safety away from the user compiling the code. Does anyone actually take advantage of this control?

I'm currently hoping to make work -boolean/-numeric with automatic detection of the need for locks in -numeric. I'd probably want to make -boolean the default. It wouldn't affect the report tool. The runtime would just output counts 0 and 1.

[rleonid]

Without quoting...

1. As far as I've tested it, yes, -fast is much faster than -safe. Around an order of magnitude (minutes -> seconds) on Oml and Ketrew. I think the -safe is mostly legacy, I've kept it around for backward compatibility. I've never found an instance where they diverged, but I've only spot checked the test suite and the projects where I've applied the switch. If you want to just have one mode that is canonical and "just works", I am all in favor of jettisoning -safe and changing -faster.
2. A performance test is a good idea, but not required. The improvement is noticeable.
3. A race condition test is probably a good idea, we had a bug dealing with OUnit threads a couple of months ago.
4. I don't know how people use the Bisect_ppx counts. I think of this as a library to help in testing infrastructure, so I don't think of it as being that performance critical, but having accurate counts as being more valuable. One certainly would want to know if a count is not what you expect (which just might be zero). My preference would be for just one mode that does the counting correctly and get rid of the different modes entirely, ie your -numeric. Perhaps we can keep the -fast mode but with separate arrays (that are created as needed) per thread, to avoid the blocking. I don't know too much about ocaml.ppx.context

[aantron]

I agree with your preference.

The one mode should be the current -faster, i.e. per-module arrays and no locks. We can maintain the other flags and just ignore them. Here is why:

Relative to current OCaml:

• There is no pre-emption AFAICT in bytecode or native code unless you allocate or do I/O, so -faster and -fast are inherently thread-safe. Ironically, -safe is the only mode that was ever unsafe, but only if you have a large number of points and have to resize arrays (haven't verified this).
• I think this is true no matter what libraries or configurations you are using.

Relative to future OCaml:

• I need to read up on this, but I think multicore will allow concurrent execution of our counting code, but still will not allow pre-emption of a thread that is already running on a core during count update. Then, as long as multicore also provides atomic compare-and-exchange, we should be able to wrap counts in spin locks, or find another method of fast mutual exclusion. Then, we should be able to make the code always thread-safe in one mode without a significant performance hit.
• In any case, we don't know exactly what there will be so I think we shouldn't try to solve this now.
• On the other hand, we are well prepared to solve it quickly when it's time, since we will have this experience and the thread safety test.

[rleonid]

Reasonable.

[aantron]

Actually, it occurs to me that running on ocamljava probably requires the locks, though I am not sure if it worked there, since BisectThread required Mutex, and I don't see Mutex in any of the docs. Thoughts?