Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make DecFP package thread safe #39

Closed
ScottPJones opened this issue May 1, 2017 · 8 comments · Fixed by #106
Closed

Make DecFP package thread safe #39

ScottPJones opened this issue May 1, 2017 · 8 comments · Fixed by #106

Comments

@ScottPJones
Copy link
Contributor

Although the multithreading support in Julia v0.6 is still classified as experimental, I assume that the goal is to have it fully working (and have packages work correctly when threads are used) by the time v1.0 is released.
Currently, the DecFP uses a single buffer (_buffer) used when converting a decimal to a string,
as well as C globals for rounding mode and exception flags.
When using threads, the buffer would need to be stored in thread-local storage or locking used when using the buffer, and the rounding mode and exception flags would need to also be in thread-local storage (and the Intel package compiled to pass those on every call instead of using global values).

@stevengj
Copy link
Member

The easiest way to fix this would be to use thread-local variables: just make an array of length Threads.nthreads() and use Threads.threadid() to index it.

@stevengj
Copy link
Member

Hmm, an underlying problem here is that it's not clear whether the libbid library itself is thread-safe.

@stevengj
Copy link
Member

Ah, yes, it looks like libbid can be configured to use thread-local variables (via __thread or __declspec(thread) for its globals). Though I'm not sure how to access these via Julia's cglobal?

Alternatively, it seems that there is a compiler option to pass these via arguments rather than using globals.

@stevengj
Copy link
Member

Okay, since cglobal calls dlsym, it looks like this should actually be fine as long as we call it separately from each thread. (dlsym works per-thread for thread-local variables, according to docs.) But it still might be preferable just to use arguments instead.

@randyzwitch
Copy link

I just ran into this as I tried to add threads to my database package. Was there any attempt to implement Steven's idea above?

@stevengj
Copy link
Member

No, I don't think anyone has recompiled DecFP using thread-local variables, which seems like the easiest approach.

@randyzwitch
Copy link

Do you happen to know what that flag would be, or can you link my to the documentation? I'm happy to try and update the BinaryBuilder package, but it's not clear to me what I should add to the script

@jmkuhn
Copy link
Contributor

jmkuhn commented Feb 11, 2020

This is a rough outline of the steps to make this package thread safe.

In https://github.com/quinnj/DecFPBuilder compile with GLOBAL_RND=0 and GLOBAL_FLAGS=0.

In DecFP.jl __init__() initialize vectors of rounding mode and exception flags with one element for each thread. See JuliaLang/julia#34546 for an example.

In DecFP.jl modify the calls into the C library to pass the thread local rounding mode and exception flags. See the Intel C library source LIBRARY/src/bid_functions.h starting at line 2994 for the function signatures.

Write multithreading tests with different rounding modes and tests to exercise different exception flags in different threads.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants