Thread safety of printing and matmul

[stevengj]

This PR uses a per-thread buffer to make the generic-matmul thread-safe (fixes #22581), floating-point printing thread-safe (fixes #25727), and printf thread-safe (fixes #21335).

In particular, the old DIGITS array in grisu is supplemented by a DIGITSs = [DIGITS] array that is expanded by __init__ into a per-thread buffer. I kept the old DIGITS array to avoid breaking the few external packages that use this — they can migrate gradually to the thread-safe version.

The basic matmul buffer structure is not changed, so #26008 is neither better nor worse.

As I mentioned in #10441, it would be good to have better support for thread-local variables in the compiler. Ideally I could just put @threadlocal const DIGITS=[....] and it would do the right thing under the hood.

[Keno]

This approach seems fine with me for now. We'll have to carefully think this through with the new threading runtime, since that'll start scheduling tasks on different threads, so thread-local will no longer be right. cc @kpamnany

[JeffBezanson]

Right, in general we need task-local storage, but I expect thread-local to be much more efficient (since there's a fixed number of threads, among other things). So we should continue to use this technique for internal buffers where we know that tasks won't block while using them (they won't, right? right? 😄 ).

[Keno]

One possible model would be to an explicit acquire/release for task local buffers, such that they get automatically copied along with the task if switching does happen.

[stevengj]

CI failures are unrelated, I think. MacOS Travis is failing with fatal error: 'cholmod.h' file not found, and 32-bit appveyor seems to be a timeout.

[JeffBezanson]

Would be good to spot check that float to string conversion and printf performance are unaffected. I expect there would be no difference, but you never know.

[stevengj]

old (master) thread-unsafe version:

julia> foo(x) = @sprintf("%3.4f %10.3g", x, x);
       foo(io, x) = @printf(io, "%3.4f %10.3g", x, x);

julia> @btime string(3.7234);
  338.630 ns (4 allocations: 192 bytes)

julia> @btime foo(3.7234);
  738.968 ns (10 allocations: 576 bytes)

julia> buf = IOBuffer(sizehint=100); @btime print($buf, 3.7234);
  255.677 ns (0 allocations: 0 bytes)

julia> buf = IOBuffer(sizehint=100); @btime foo($buf, 3.7234);
  717.572 ns (5 allocations: 288 bytes)

new thread-safe version:

julia> @btime string(3.7234);
  332.072 ns (4 allocations: 192 bytes)

julia> @btime foo(3.7234);
  760.429 ns (10 allocations: 576 bytes)

julia> buf = IOBuffer(sizehint=100); @btime print($buf, 3.7234);
  257.676 ns (0 allocations: 0 bytes)

julia> buf = IOBuffer(sizehint=100); @btime foo($buf, 3.7234);
  745.630 ns (5 allocations: 288 bytes)

So there seems to be at most a 4% slowdown, usually less. This seems a worthwhile price to pay for thread safety.

If you care about @printf performance, the most important thing is perhaps getting rid of the 5 allocations when writing to a pre-allocated buffer, which pre-date this PR.

[stevengj]

Okay to merge?

[vtjnash]

access to DIGITSs is inbounds

I think I would prefer we drop this commit unless it is absolutely conclusive that it is essential. We can refactor some of this code to move the computation of the buffer outside hot loops if necessary.

[stevengj]

@vtjnash, reverted. I just tested and it makes no measurable difference in performance.

[stevengj]

Unrelated timeout error on AppVeyor 64-bit

[simonbyrne]

Do we need to do the same thing for Grisu.BIGNUMS as well?

@quinnj do you know?

[quinnj]

Hmmmm, not sure. I think I remember @vtjnash fixing that a while ago though.

[simonbyrne]

No, I managed to trigger an issue. See #27826