Use TBB's concurrent queue implementation #158
Conversation
|
@jamesturner246 Yep. It will behave in exactly the same way. |
| find_package(fmt CONFIG REQUIRED) | ||
| find_package(cxxopts CONFIG REQUIRED) | ||
| find_package(nlohmann_json CONFIG REQUIRED) | ||
| find_package(TBB CONFIG REQUIRED) | ||
| find_path(RAPIDCSV_INCLUDE_DIRS "rapidcsv.h") | ||
| find_package(Threads REQUIRED) |
There was a problem hiding this comment.
Knowing nothing about CMake, I'm curious: where does CMake looks for all of these packages?
There was a problem hiding this comment.
Traditionally, CMake looks for these on your system and e.g. the TBB library will come with a CMake package config file. And traditionally this worked rather poorly on Windows 😛.
Health-GPS uses vcpkg on top to manage dependencies so they are automatically built from source before everything else is (provided you have vcpkg installed!).
|
TBB is Intel's Threading Building Blocks library: https://github.com/oneapi-src/oneTBB It basically just provides a bunch of useful bits for multithreaded applications, including tools for parallelising work and threadsafe containers (which I'm using here). Newer versions of the C++ standard include parallelised versions of common algorithms (e.g. sort) and gcc's implementation just uses TBB under the hood, so we need it for that anyway, so it kind of makes sense to just have it as a "proper" dependency and then we can use some of the nice features on all platforms. |
TBB is a C++ library for multithreading. Currently we do use it in the repo, but only indirectly: gcc uses it as its backend for the STL parallel algorithms, so you need it on Linux if you actually want the algorithms to run in parallel. The thing is that you have to remember to install it manually. It would be easier to just have it installed with
vcpkgalong with the other dependencies. This does mean that it would therefore become a dependency for Windows too, but it's not a big one (< 2MiB on my machine). The other advantage of making TBB a requirement is that we can make use of some of its features. Notably, it contains threadsafe versions of various containers, which, aside from being safer than doing the locking yourself, are also often faster, so we could use these in many places in the code base (#159). Currently the CPU usage is poor, which might be due to lock contention (#80) and in that case, using TBB's containers would likely help.This PR makes TBB a hard requirement and integrates it with
vcpkg. I also replaced the customThreadsafeQueueimplementation withtbb::concurrent_queueand removed the former from the code base (fixes #151).The motivation for this PR was that I was getting failures for the
ThreadsafeQueuetests on macOS runners. After looking at the tests, it seems that the problem was probably just that the tests relied on things happening within a certain timeframe (eww), but rather than trying to fix the tests, I thought it was better just to rip this class out altogether as it was on my hit list anyway.