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

threads: expand thread-safe region for I/O #32309

Merged
merged 1 commit into from Jun 21, 2019
Merged

threads: expand thread-safe region for I/O #32309

merged 1 commit into from Jun 21, 2019

Conversation

@vtjnash
Copy link
Member

vtjnash commented Jun 12, 2019

This should hopefully cover most I/O operations that go through libuv (meaning that other operations like using IOStream files aren't part of this PR), to make them thread-safe.

The UDPSocket implementation of many things has been always wrong, this also fixes them.

@vtjnash vtjnash requested a review from JeffBezanson Jun 12, 2019
@vtjnash vtjnash force-pushed the jn/thread-io branch from 6a7d93a to f5b7b25 Jun 14, 2019
@musm

This comment has been minimized.

Copy link
Contributor

musm commented Jun 14, 2019

Does this make function such as uv_os_homedir which libuv states are not thread safe, thread-safe in Julia?

@vtjnash

This comment has been minimized.

Copy link
Member Author

vtjnash commented Jun 14, 2019

I wouldn't rely too closely on the libuv docs 😜. I think that warning seems to have gotten fixed before the PR was merged. I'm not sure why the message got left in. Perhaps you'd like to delete it for them?

@vtjnash vtjnash force-pushed the jn/thread-io branch from 3103f43 to 9e6eb70 Jun 16, 2019
@vtjnash vtjnash marked this pull request as ready for review Jun 18, 2019
@vtjnash vtjnash force-pushed the jn/thread-io branch from 9e6eb70 to 7875088 Jun 18, 2019
This should hopefully cover most I/O operations which go through libuv to make them thread-safe.

There is no scaling here, just one big global lock, so expect
worse-than-single-threaded performance if doing I/O on multiple threads (as
compared to doing the same work on one thread). The intention is to handle
performance improvement incrementally later.

It also necessarily redesigns parts of the UDPSocket implementation
to properly handle concurrent (single-threaded) usage, as a necessary
part of making it handle parallel (thread-safe) usage.
@JeffBezanson JeffBezanson force-pushed the jn/thread-io branch from 7875088 to cfd2a14 Jun 21, 2019
@JeffBezanson JeffBezanson merged commit 58bafe4 into master Jun 21, 2019
1 of 9 checks passed
1 of 9 checks passed
buildbot/llvmpasses_linux64 Run started
Details
buildbot/package_freebsd64 Run started
Details
buildbot/package_linux32 Run started
Details
buildbot/package_macos64 Run started
Details
buildbot/package_win32 Run started
Details
buildbot/package_win64 Run started
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
buildbot/whitespace_linux32 Run complete
Details
@JeffBezanson JeffBezanson deleted the jn/thread-io branch Jun 21, 2019
vtjnash added a commit that referenced this pull request Jun 26, 2019
After #32309, we became much more eager to forget about normal errors
and more eager to throw an EOFError at the wrong place.
This is intended to fix that oversight.
vtjnash added a commit that referenced this pull request Jun 27, 2019
After #32309, we became much more eager to forget about normal errors
and more eager to throw an EOFError at the wrong place.
This is intended to fix that oversight.
vtjnash added a commit that referenced this pull request Jul 11, 2019
After #32309, we became much more eager to forget about normal errors
and more eager to throw an EOFError at the wrong place.
This is intended to fix that oversight.
@PerezHz PerezHz mentioned this pull request Sep 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.