Release the GIL around the TA-Lib C calls - #763
Open
meet-brad-ch wants to merge 1 commit into
Open
meet-brad-ch wants to merge 1 commit into
meet-brad-ch wants to merge 1 commit into
Conversation
Every generated wrapper runs its one C call inside `with nogil:`, so calls from several threads run concurrently. The declarations carry `nogil` on the ta_func.h block, the two generators emit the block, and _ta_lib.c is regenerated with Cython 3.2.8. Input checks, output allocation and the return-code check stay under the GIL. Initialization, shutdown and the global settings stay under the GIL too; the README states the model. A new test runs seven functions across the three APIs from eight threads and checks every result against the single-threaded one.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Every generated wrapper in
_func.pxiand_stream.pxinow runs its one C call insidewith nogil:. The declarations in_ta_lib.pxdcarrynogilon theta_func.hblock so Cython allows it. The two generators emit the block, so the nextmake generatekeeps it. Nothing else changes: input checks, output allocation, the NaN prefix and the return-code check still run under the GIL, before and after the call.This is the change asked for in #128 in 2016 and in the recent discussion on the TA-Lib side: TA-Lib/ta-lib#414 (comment).
Why it is safe
<double *>(real.data)+begidx, the cleaned parameter names,&outbegidx,&outnbelement, and the output pointers. I checked the generated code for single-output, multi-output (MACD), integer-output (CDLDOJI), array-parameter (MAVP) and streaming functions.ndarray.datais declared in Cython's numpy.pxdascdef inline char* data(self) nogiland compiles toPyArray_BYTES, a struct read. The generated C forTA_SMAisTA_SMA(0, endidx, ((double *)__pyx_f_5numpy_7ndarray_4data_data(real)) + begidx, ...), with no Python API call in between.with nogil.ta_global.hstates that init and shutdown are multithread-protected, and the indicator functions only read the global settings.Threading model, now stated in the README: indicator calls may run concurrently from any number of threads.
_ta_initialize,_ta_shutdown,set_unstable_period,set_compatibility,_ta_set_candle_settingsand_ta_restore_candle_default_settingsstay under the GIL. Changing a setting while calls run in other threads is undefined behavior.Test:
tests/test_threads.pyruns EMA, RSI, MACD, CDLDOJI, ATR, the streaming EMA and the abstract BBANDS from 8 threads for 50 rounds each. It checks every result against the single-threaded one withassert_array_equal. A second test changes an unstable period between two concurrent batches and checks that every later call sees it.Numbers, same machine (24 threads, Windows, Python 3.12, numpy 2.5, polars 1.44), median of 7:
map_batches(talib.EMA), 8 indicators in one select, 1M rowsmap_batches(talib.EMA), 8 indicators in one select, 10M rowsSingle calls do not change. Anything that calls from several threads now overlaps.
About the diff
talib/_ta_lib.cis regenerated with Cython 3.2.8, the version of the committed file. Its diff is large for two reasons. Every function grew by one line and Cython tracks line numbers, and each of the 322 calls gained the GIL release and acquire._func.pxiand_stream.pxicarry exactly the transformation the generators now emit, applied to the committed files. Runningmake generateproduces the same content and additionally swaps the order of AVGDEV and AVGPRICE. That swap predates this change and is left out to keep the diff readable.nogilmarkers thatTA_RSIandTA_RSI_Lookbackalready had are removed, since the whole block now carries it.