VSadov / NonBlocking

Implementation of a lock-free dictionary on .Net.
MIT License
488 stars 34 forks source link

Tests should use threads, not tasks #7

Closed nerai closed 2 years ago

nerai commented 4 years ago

As far as I'm aware, tasks are not guaranteed to run asynchronously. Since the tests use tasks (Task.Run ...) instead of threads, it could happen (unlikely) that they run entirely in order, not in a parallel (multiple cores) or simulated-parallel (concurrent) fashion. To ensure a sufficiently adverse environment, the tests should use threads instead of tasks.

VSadov commented 4 years ago

Task.Factory.StartNew(body, TaskCreationOptions.LongRunning) starts a new thread because of LongRunning option.

Here we create threads:

            for (int i = 0; i < workers.Length; i++)
            {
                workers[i] = Task.Factory.StartNew(body, TaskCreationOptions.LongRunning);
            }

            stop_time = sw.ElapsedMilliseconds + time;

And here execution starts in all workers (which are all waiting for this event):

            e.Set();

The latency of an event is orders of magnitude faster than duration of one benchmark (3sec.), so when a benchmark needs a particular degree of concurrency the above guarantees it quite reliably.

I do not think we have a problem here.

======= relevant code here:

https://github.com/VSadov/NonBlocking/blob/6c9824fef8c92de82970e020ecda6ed234c8615c/BenchmarksCore/Program.cs#L391

VSadov commented 4 years ago

For the tests - by default they run continuously and could be left running for hours.

While thread pool does not guarantee a particular concurrency mode, it appears to expose tests to all kind of random combinations (sequential or concurrent) and in practice catches problems really well.

nerai commented 4 years ago

Regarding LongRunning, I don't think the behavior of creating a new, separate thread is guaranteed:

LongRunning: Specifies that a task will be a long-running, coarse-grained operation involving fewer, larger components than fine-grained systems. It provides a hint to the TaskScheduler that oversubscription may be warranted. Oversubscription lets you create more threads than the available number of hardware threads. It also provides a hint to the task scheduler that an additional thread might be required for the task so that it does not block the forward progress of other threads or work items on the local thread-pool queue. [src]

VSadov commented 2 years ago

Tasks appear to work as expected. I do not think we have a problem here.