This is well-written. I could follow along quite nicely, from the setup through the bottlenecks and onto the resolution of the performance bug. Even the PRs are very pleasant to read: the majority of them is just a handful of changed lines with an added tests and a bit of documentation.
I was taken aback for a moment that this work originated from a report on StackOverflow. I had thought SO was effectively dead and abandoned by its community. But maybe I shouldn't project my own experience onto everyone else.
I thought NumPy was already releasing the GIL. On regular non-free-threaded Python, you can run threaded parallel Numpy operations and have multiple cores doing 100%, I've relied on that. Maybe not the case with the operations this article focuses on (sin/cos).
> only acquire the lock when the flag needs to be updated
Unclear why you still need the lock here in that case. The idea that this flag may get updated during runtime and impacts how the software works when set seems to clash with the idea we need take no action having performed a relaxed (ie non-synchronising) load and seen it wasn't set at some previous time.
Maybe there's something I don't understand about these internals, which may be as simple as "It's just advisory so if we don't trace when we should no big deal".
I know this is more or less expected, but the improvement induced by adding a worker diminishes very rapidly... I guess it's not the cpython/numpy's fault but rather the CPU.
The plot doesn't appear to be in Amdahl territory yet. The single-threaded time in the plot looks to be around 39 seconds. A perfect division into 32 workers without overhead would make it 39 / 32 = 1.22 seconds. With the multi-threaded workload being reported as 1.5 seconds in the text, there's still only .3 seconds of overhead + serial instructions that can't be parallelized.
Every doubling of the number of workers halves the execution time cleanly in the plot, from 40 seconds to 20 seconds to 10 seconds. Eyeballing this for 32 over 16 workers is difficult, but it still seems close to halving the total time once again. So there's not a lot of Amdahl flattening, it's just the plain physics of looking at a inverse-proportional curve.
This is well-written. I could follow along quite nicely, from the setup through the bottlenecks and onto the resolution of the performance bug. Even the PRs are very pleasant to read: the majority of them is just a handful of changed lines with an added tests and a bit of documentation.
I was taken aback for a moment that this work originated from a report on StackOverflow. I had thought SO was effectively dead and abandoned by its community. But maybe I shouldn't project my own experience onto everyone else.
I thought NumPy was already releasing the GIL. On regular non-free-threaded Python, you can run threaded parallel Numpy operations and have multiple cores doing 100%, I've relied on that. Maybe not the case with the operations this article focuses on (sin/cos).
> only acquire the lock when the flag needs to be updated
Unclear why you still need the lock here in that case. The idea that this flag may get updated during runtime and impacts how the software works when set seems to clash with the idea we need take no action having performed a relaxed (ie non-synchronising) load and seen it wasn't set at some previous time.
Maybe there's something I don't understand about these internals, which may be as simple as "It's just advisory so if we don't trace when we should no big deal".
Nice to see the performance improvements work.
I know this is more or less expected, but the improvement induced by adding a worker diminishes very rapidly... I guess it's not the cpython/numpy's fault but rather the CPU.
The more fundamental reason is Amdahl's law
https://en.wikipedia.org/wiki/Amdahl%27s_law
Even a tiny bit of serial instruction will limit the speed up
The plot doesn't appear to be in Amdahl territory yet. The single-threaded time in the plot looks to be around 39 seconds. A perfect division into 32 workers without overhead would make it 39 / 32 = 1.22 seconds. With the multi-threaded workload being reported as 1.5 seconds in the text, there's still only .3 seconds of overhead + serial instructions that can't be parallelized.
Every doubling of the number of workers halves the execution time cleanly in the plot, from 40 seconds to 20 seconds to 10 seconds. Eyeballing this for 32 over 16 workers is difficult, but it still seems close to halving the total time once again. So there's not a lot of Amdahl flattening, it's just the plain physics of looking at a inverse-proportional curve.
Great work!