mininode: add an optimistic write and disable nagle #11323
Merged
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
Because the poll/select loop may pause for 100msec before actually doing a send, and we have no way to force the loop awake, try sending from the calling thread if the queue is empty. Also, disable nagle as all sends should be either full messages or unfinished sends. This shaves an average of ~1 minute or so off of my accumulated runtime, and 10-15 seconds off of actual runtime.
|
Mega concept ACK! Tested ACK 1817398. I've run the extended test suite over this locally and it passes. It makes any test using the p2p interface much faster. Travis has intermittent timing issues, so we'd need to make sure this is well tested in that environment before merging. |
jimpo
reviewed
Sep 14, 2017
| @@ -1792,7 +1793,14 @@ def send_message(self, message, pushbuf=False): | |||
| tmsg += h[:4] | |||
| tmsg += data | |||
| with mininode_lock: | |||
| self.sendbuf += tmsg | |||
| if (len(self.sendbuf) == 0 and not pushbuf): | |||
There was a problem hiding this comment.
nit: if not self.sendbuf and not pushbuf:. From PEP8: "For sequences, (strings, lists, tuples), use the fact that empty sequences are false."
There was a problem hiding this comment.
No need to fix this "style nit". len(some_list)==0 is stricter than just casting any object/primitive type to boolean. Also it is more clear to read, imo.
There was a problem hiding this comment.
@MarcoFalke I believe that for values of the list type if len(some_list)==0 is actually identical to if not some_list, and in general I'd argue for following things like PEP8.
There was a problem hiding this comment.
ACK with or without this nit change. I have a slight preference for being pythonic and following PEP8. Leaving the longer form (if (len(self.sendbuf) ==0 would catch any bugs where someone set self.sendbuf to something other than a list, but I don't think that's a good justification.
There was a problem hiding this comment.
Either way I think the brackets should at least be removed
There was a problem hiding this comment.
@sipa, it is identical. Though, in the case that some_list is False or None or 0 or anything else that casts to False, len(some_list) will fail appropriately. I don't think it is a downside to fail in the test framework when a parameter has the wrong type.
There was a problem hiding this comment.
I think to be hyper pedantic, if len(self.sendbuf) == 0 is equivalent to:
if not hasattr(self.sendbuf, '__len__'):
raise TypeError
else if not self.sendbuf:
# do stuffie, it basically has an implicit assert that the type of self.sendbuf has a length property.
In general we don't litter the Python code with type checks, even though it's not a type safe language and someone could call into functions with the wrong typed parameters.
There was a problem hiding this comment.
👍 on @jimpo and @meshcollider suggestions.
|
Cory is being modest as usual. Let's run this over just the tests that use the p2p network thread. master: this branch: That's a ~25% reduction in cumulative test time, with particularly impressive reduction in |
|
utACK |
|
Nice, Concept ACK |
|
I'll defer on the style nit, I'm not familiar enough with python to have a preference. |
|
Nice! |
|
utACK 1817398. Not going to hold this up on style nits, as there are other places where this is "wrong" and we don't enforce any style in the python code as of now. |
maflcko
pushed a commit
that referenced
this pull request
Sep 18, 2017
1817398 mininode: add an optimistic write and disable nagle (Cory Fields) Pull request description: Disclaimer: I'm not familiar with asyncore, so I'm unclear how safe this is. It works for me (tm). Because the poll/select loop may pause for 100msec before actually doing a send, and we have no way to force the loop awake, try sending from the calling thread if the queue is empty. Also, disable nagle as all sends should be either full messages or unfinished sends. This shaves an average of ~1 minute or so off of my accumulated runtime, and 10-15 seconds off of actual runtime. Tree-SHA512: 6b61b8058e621dacf0b4dd353c10e3666fbda0691440eb6ebc432491ebada80a781dcd09291bf03e70112a41d3c2a0c91775ed08824b79bf8d0ebed11595c28b
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Dec 22, 2019
1817398 mininode: add an optimistic write and disable nagle (Cory Fields) Pull request description: Disclaimer: I'm not familiar with asyncore, so I'm unclear how safe this is. It works for me (tm). Because the poll/select loop may pause for 100msec before actually doing a send, and we have no way to force the loop awake, try sending from the calling thread if the queue is empty. Also, disable nagle as all sends should be either full messages or unfinished sends. This shaves an average of ~1 minute or so off of my accumulated runtime, and 10-15 seconds off of actual runtime. Tree-SHA512: 6b61b8058e621dacf0b4dd353c10e3666fbda0691440eb6ebc432491ebada80a781dcd09291bf03e70112a41d3c2a0c91775ed08824b79bf8d0ebed11595c28b
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jan 2, 2020
1817398 mininode: add an optimistic write and disable nagle (Cory Fields) Pull request description: Disclaimer: I'm not familiar with asyncore, so I'm unclear how safe this is. It works for me (tm). Because the poll/select loop may pause for 100msec before actually doing a send, and we have no way to force the loop awake, try sending from the calling thread if the queue is empty. Also, disable nagle as all sends should be either full messages or unfinished sends. This shaves an average of ~1 minute or so off of my accumulated runtime, and 10-15 seconds off of actual runtime. Tree-SHA512: 6b61b8058e621dacf0b4dd353c10e3666fbda0691440eb6ebc432491ebada80a781dcd09291bf03e70112a41d3c2a0c91775ed08824b79bf8d0ebed11595c28b
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jan 4, 2020
1817398 mininode: add an optimistic write and disable nagle (Cory Fields) Pull request description: Disclaimer: I'm not familiar with asyncore, so I'm unclear how safe this is. It works for me (tm). Because the poll/select loop may pause for 100msec before actually doing a send, and we have no way to force the loop awake, try sending from the calling thread if the queue is empty. Also, disable nagle as all sends should be either full messages or unfinished sends. This shaves an average of ~1 minute or so off of my accumulated runtime, and 10-15 seconds off of actual runtime. Tree-SHA512: 6b61b8058e621dacf0b4dd353c10e3666fbda0691440eb6ebc432491ebada80a781dcd09291bf03e70112a41d3c2a0c91775ed08824b79bf8d0ebed11595c28b
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jan 12, 2020
1817398 mininode: add an optimistic write and disable nagle (Cory Fields) Pull request description: Disclaimer: I'm not familiar with asyncore, so I'm unclear how safe this is. It works for me (tm). Because the poll/select loop may pause for 100msec before actually doing a send, and we have no way to force the loop awake, try sending from the calling thread if the queue is empty. Also, disable nagle as all sends should be either full messages or unfinished sends. This shaves an average of ~1 minute or so off of my accumulated runtime, and 10-15 seconds off of actual runtime. Tree-SHA512: 6b61b8058e621dacf0b4dd353c10e3666fbda0691440eb6ebc432491ebada80a781dcd09291bf03e70112a41d3c2a0c91775ed08824b79bf8d0ebed11595c28b
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jan 12, 2020
1817398 mininode: add an optimistic write and disable nagle (Cory Fields) Pull request description: Disclaimer: I'm not familiar with asyncore, so I'm unclear how safe this is. It works for me (tm). Because the poll/select loop may pause for 100msec before actually doing a send, and we have no way to force the loop awake, try sending from the calling thread if the queue is empty. Also, disable nagle as all sends should be either full messages or unfinished sends. This shaves an average of ~1 minute or so off of my accumulated runtime, and 10-15 seconds off of actual runtime. Tree-SHA512: 6b61b8058e621dacf0b4dd353c10e3666fbda0691440eb6ebc432491ebada80a781dcd09291bf03e70112a41d3c2a0c91775ed08824b79bf8d0ebed11595c28b
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jan 12, 2020
1817398 mininode: add an optimistic write and disable nagle (Cory Fields) Pull request description: Disclaimer: I'm not familiar with asyncore, so I'm unclear how safe this is. It works for me (tm). Because the poll/select loop may pause for 100msec before actually doing a send, and we have no way to force the loop awake, try sending from the calling thread if the queue is empty. Also, disable nagle as all sends should be either full messages or unfinished sends. This shaves an average of ~1 minute or so off of my accumulated runtime, and 10-15 seconds off of actual runtime. Tree-SHA512: 6b61b8058e621dacf0b4dd353c10e3666fbda0691440eb6ebc432491ebada80a781dcd09291bf03e70112a41d3c2a0c91775ed08824b79bf8d0ebed11595c28b
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jan 12, 2020
1817398 mininode: add an optimistic write and disable nagle (Cory Fields) Pull request description: Disclaimer: I'm not familiar with asyncore, so I'm unclear how safe this is. It works for me (tm). Because the poll/select loop may pause for 100msec before actually doing a send, and we have no way to force the loop awake, try sending from the calling thread if the queue is empty. Also, disable nagle as all sends should be either full messages or unfinished sends. This shaves an average of ~1 minute or so off of my accumulated runtime, and 10-15 seconds off of actual runtime. Tree-SHA512: 6b61b8058e621dacf0b4dd353c10e3666fbda0691440eb6ebc432491ebada80a781dcd09291bf03e70112a41d3c2a0c91775ed08824b79bf8d0ebed11595c28b
gades
pushed a commit
to cosanta/cosanta-core
that referenced
this pull request
Jun 30, 2021
1817398 mininode: add an optimistic write and disable nagle (Cory Fields) Pull request description: Disclaimer: I'm not familiar with asyncore, so I'm unclear how safe this is. It works for me (tm). Because the poll/select loop may pause for 100msec before actually doing a send, and we have no way to force the loop awake, try sending from the calling thread if the queue is empty. Also, disable nagle as all sends should be either full messages or unfinished sends. This shaves an average of ~1 minute or so off of my accumulated runtime, and 10-15 seconds off of actual runtime. Tree-SHA512: 6b61b8058e621dacf0b4dd353c10e3666fbda0691440eb6ebc432491ebada80a781dcd09291bf03e70112a41d3c2a0c91775ed08824b79bf8d0ebed11595c28b
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Disclaimer: I'm not familiar with asyncore, so I'm unclear how safe this is. It works for me (tm).
Because the poll/select loop may pause for 100msec before actually doing a send, and we have no way to force the loop awake, try sending from the calling thread if the queue is empty.
Also, disable nagle as all sends should be either full messages or unfinished sends.
This shaves an average of ~1 minute or so off of my accumulated runtime, and 10-15 seconds off of actual runtime.