Add concurrent tree build support

[Mintpasokon]

Related issue

#165

Problem with helgrind

When tested with valgrind --tool=helgrind tests/unit_tests, the output is like:

==551969== Possible data race during write of size 8 at 0x4DF8E48 by thread #4
==551969== Locks held: none
==551969==    at 0x4858B70: memset (in /usr/libexec/valgrind/vgpreload_helgrind-amd64-linux.so)
==551969==    by 0x4C49017: get_cached_stack (allocatestack.c:138)
==551969==    by 0x4C49017: allocate_stack (allocatestack.c:364)
==551969==    by 0x4C49017: pthread_create@@GLIBC_2.34 (pthread_create.c:647)
...

Then I checked source code of glibc, and get

memset (dtv, '\0', (dtv[-1].counter + 1) * sizeof (dtv_t));

at the line mentioned in helgrind. This line of code belongs to the function used to create threads in GLIBC's nptl module, so it looks like helgrind detected it during a concurrent call to std::async. In my test, the warning can be supressed by locking the std::async like:

lock.lock();
left_future = std::async(...);
lock.unlock();

You can also reproduce the problem with helgrind from the following code:

    auto create_threads = []()
    {
        std::future<void> future[10];
        for(int t = 0; t < 10; ++t){
            future[t] = std::async(std::launch::async, [](){ int i = 0; ++i; });
        }
        for(int t = 0; t < 10; ++t){
            future[t].get();
        }
    };
    auto future1 = std::async(std::launch::async, create_threads);
    auto future2 = std::async(std::launch::async, create_threads);
    future1.get();
    future2.get();

So yes, the warning of helgrind is false positive, helgrind just warns when you create threads concurrently with glibc.

Performance

The performance improvement depends on the number of threads. In my tests, concurrent builds with 8-16 threads can improve building performance by up to around 3x.

[jlblancoc]

Great research, thanks @Mintpasokon !

PS: I forgot enabling CI checks to this repo (!), so I now got this error in one of the examples, apparently from the now-not possible "operator =" for a KD-tree index.
If possible, please take a look and try to propose a solution, otherwise I'll look at it anytime soon.
Cheers,
JL

[jlblancoc]

PS: One workaround I used to similar problems in the past is using a std::unique_ptr to the entity not having a valid operator=, but then we must also redefine the = operators, copy constructor, etc. etc. in the class having such member. Hopefully there's something easier...

[Mintpasokon]

Thanks for your advise! The CI error occurs in Ubuntu 20.04 with g++ 9, so I tested with g++ 9 (error), 10 (error), 11 (success) and 12 (success). It seems that the problem locates in this line, where

std::atomic<unsigned int> thread_count = 0;

would calls deleted

atomic& operator=(const atomic&) = delete;

instead of

constexpr atomic(__integral_type __i) noexcept : __base_type(__i) { }

before g++ 11.
To solve this problem, we can simply change std::atomic<unsigned int> thread_count = 0; to std::atomic<unsigned int> thread_count(0u);. In my tests it is built successfully in g++ 9, 10, 11 and 12.