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

TCP comms could be faster #1477

Closed
pitrou opened this issue Oct 18, 2017 · 10 comments
Closed

TCP comms could be faster #1477

pitrou opened this issue Oct 18, 2017 · 10 comments
Labels
needs info Needs further information from the user performance

Comments

@pitrou
Copy link
Member

pitrou commented Oct 18, 2017

This issue is the distributed-specific side of tornadoweb/tornado#2147. We are using IOStream for the framed Comm protocol, which incurs additional copies.

A benchmark script is available at https://gist.github.com/pitrou/245e24de52dec34e03cfc4148c001466

@pitrou
Copy link
Member Author

pitrou commented Oct 19, 2017

Some results for the above benchmark:

All this on Ubuntu 16.04 with Python 3.6.

@TomAugspurger
Copy link
Member

Was this issue essentially fixed by the use of read_into, or are there additional steps we should take?

if PY3 and self._iostream_has_read_into:
frame = bytearray(length)
n = yield stream.read_into(frame)
assert n == length, (n, length)

@pitrou
Copy link
Member Author

pitrou commented Jan 24, 2019

Both read_into and the write improvements. It should probably be fixed (out of curiosity, did you run the benchmark script and which numbers did you get?).

@TomAugspurger
Copy link
Member

633 MB/s with tornado 5.1.1

I'm working on UCX-backed comms (#2344) and will post the performance results once I get things working.

@pitrou
Copy link
Member Author

pitrou commented Jan 24, 2019

Which CPU and system, out of curiosity?

@TomAugspurger
Copy link
Member

That's on an Nvidia DGX system, and I believe the CPU is

model name      : Intel(R) Xeon(R) CPU E5-2698 v4 @ 2.20GHz

@pitrou
Copy link
Member Author

pitrou commented Jan 24, 2019

I see, thanks. My benchmark numbers above had been obtained with a Core i5-2500K.

@jakirkham
Copy link
Member

Should we close this issue? Sounds like this was already addressed (though please correct me if not)

@GenevieveBuckley GenevieveBuckley added performance needs info Needs further information from the user labels Oct 18, 2021
@GenevieveBuckley
Copy link
Contributor

Was this issue essentially fixed by the use of read_into, or are there additional steps we should take?

if PY3 and self._iostream_has_read_into:
frame = bytearray(length)
n = yield stream.read_into(frame)
assert n == length, (n, length)

@pitrou did this sufficiently fix your issues?

@jakirkham
Copy link
Member

Yeah I think this was fixed a while ago. Let’s go ahead and close this. There are already newer issues on improving communication. We can follow up on additional tasks in those issues or in a new issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs info Needs further information from the user performance
Projects
None yet
Development

No branches or pull requests

4 participants