-
Notifications
You must be signed in to change notification settings - Fork 159
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Add incremental updating of open streams count and closed_streams state #1185
base: master
Are you sure you want to change the base?
Conversation
quadratic performance with large number of open streams
On the old code, here is how
New code:
Note that for this example, I commented out the second "test" in the As you can see, this is a 36X perf improvement |
So the lints I can clean up (I assume I can just run autopep8 or something similar?), but the code coverage tests are failing for some versions of python for these lines:
Pasting the code here for reference:
Which is odd, since I can insert a print statement there and verify that the code is getting called, not to mention the counts of open outbound/inbound streams would be completely wrong if that code wasn't getting called. Is this a quirk with the code coverage tool? |
Oh I see, it's because the if() never evaluates to False. If I remove the conditional the coverage tests pass The reason that conditional is there is for defensive reasons. A function wrapped by I can write a specific test to exercise this behavior for that conditional |
Thanks, I've managed to find time to understand the problem - but I'm not sure about the solution. It would be helpful if @Lukasa could comment on the general solution and how it fits in with the codebase. I'd then be happy to comment on the details. |
I'm very glad someone already thought that this open stream count may be a problem 😍 However the The open stream count is essentially a cache; could it be implemented in some other way? |
This fixes #1184