Shorten duration while base connection locks are held #43
Conversation
This file contains 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
This includes debug logging because we really need to test this in a real system and see that we don't get errors when reading and writing.
The worst case scenario (assuming that Pipe is thread safe) is that 65537 threads call #unblock and don't see the updated value of @State and we get an exception, but we catch that exception so it's not catastrophic.
The only thread that calls #read is the IO reactor thread, so locking is unnecessary. There is a hidden assumption in this: the unblocker socket MUST be the first socket to be read after the IO reactor comes out of select, otherwise unblocks might be missed.
All the other uses of the lock have been removed, so what would this lock lock against? #close will only be called from the IO reactor thread.
This, along with a test, will make it clear that the unblocker must be the first readable passed to select.
| # | ||
| # This method differs from #cheap_peek in that it might return an empty | ||
| # result even when the buffer is non-empty. It is primarily useful when | ||
| # feeding bytes to a socket from a single reactor thread. |
There was a problem hiding this comment.
I think this description should make it more clear that a single reader is assumed (for example: "It is only safe when there is only one reader, and it's primarily useful when feeding bytes to a socket from a single reactor thread").
|
👍 |
grddev
force-pushed
the
improve-base-connection-locks
branch
from
0f5a487
to
009f4a5
Compare
This avoids performing the nonblocking write while holding the lock. While it seems counter-intuitive, it seems this operation frequently takes a lot of time. I don't know whether it is due to a profiling artifact or not, but it shouldn't hurt in any case. Since reads from the write buffer only happen from the single reactor thread, we could actually perform (some) flushes without locking before performing the non-blocking write. This uses a new byte-buffer accessor that allows peeking at the contents of the byte buffer without taking any lock.
iconara
force-pushed
the
improve-base-connection-locks
branch
from
009f4a5
to
0fc72e5
Compare
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.
See individual commits for more details.