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

fix #29885, make grisu digit buffer task-local #29907

Merged
merged 1 commit into from Nov 28, 2018
Merged

Conversation

JeffBezanson
Copy link
Sponsor Member

@JeffBezanson JeffBezanson commented Nov 2, 2018

I thought task-local storage might be too slow for this, but fortunately the slowdown is only about 5-6%. I also tried per-thread pools but it wasn't faster. Hopefully TLS access can be sped up a bit too.

Note: I kept the existing global DIGITS in place since I believe some packages use it.

fix #29885

@JeffBezanson JeffBezanson added I/O Involving the I/O subsystem: libuv, read, write, etc. bugfix This change fixes an existing bug labels Nov 2, 2018
@async print(q, 9.8)
read(p, 2^18)
@test read(p, String) == "12.345"
end
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrap in @sync (so we'll collect any errors)

@simonbyrne
Copy link
Contributor

I think we need to do the same thing for BIGNUMS as well (it's harder to trigger, but maybe see #27826).

@JeffBezanson
Copy link
Sponsor Member Author

Do task-blocking operations happen while BIGNUMS is still in use?

@simonbyrne
Copy link
Contributor

What operations are task-blocking?

@JeffBezanson
Copy link
Sponsor Member Author

I/O, sleep, calls to wait or fetch, yield.

@JeffBezanson JeffBezanson added the needs news A NEWS entry is required for this change label Nov 27, 2018
@JeffBezanson
Copy link
Sponsor Member Author

Will add news separately.

@JeffBezanson JeffBezanson merged commit a50d5af into master Nov 28, 2018
@JeffBezanson JeffBezanson deleted the jb/fix29885 branch November 28, 2018 20:23
@JeffBezanson JeffBezanson removed the needs news A NEWS entry is required for this change label Nov 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug I/O Involving the I/O subsystem: libuv, read, write, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DIGITS buffer may be reused while in-use
4 participants