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

Printing progress on add significantly slows down add #4297

Open
Stebalien opened this issue Oct 11, 2017 · 8 comments
Open

Printing progress on add significantly slows down add #4297

Stebalien opened this issue Oct 11, 2017 · 8 comments

Comments

@Stebalien
Copy link
Member

With #4296 applied, redirecting stdout to /dev/null speeds up add by ~17%. Something is waiting on this progress output that shouldn't be (we should be using a "lossy" channel to return progress updates, dropping any progress updates the client isn't ready for).

@Stebalien
Copy link
Member Author

Hm. Actually, we can't do this because we send back all blocks added, not just progress updates. A CBOR API may help a bit but...

@kevina
Copy link
Contributor

kevina commented Oct 11, 2017

@Stebalien I have not even looked at the code yet, but would increasing the buffer size help?

@Stebalien
Copy link
Member Author

Probably not (it'll just fill up).

After looking at it, we can, for larger files, skip progress updates if the buffer is full (I believe). Also, we can avoid blocking on printing updates (I'm working on that now).

@Stebalien
Copy link
Member Author

Also, we don't buffer stdout/stderr. We should consider using double-buffering (with separate IO go routines for flushing).

@kevina
Copy link
Contributor

kevina commented Oct 11, 2017

@Stebalien see requirement (6) in #3856 (comment):

  1. Ideally, the command code should not have to worry about connection errors, unless it needs to abort. That basically means that it should be able to just blindly send data, and errors, and only have to check if it should abort.

@Stebalien
Copy link
Member Author

@kevina I don't see the connection. This is just an issue about perf (and dealing with the fact that stderr/stdout is slow).

@kevina
Copy link
Contributor

kevina commented Oct 12, 2017

@Stebalien part of the idea behind (6) is that the client should never block the server just because it is not ready to receive more output

@Stebalien
Copy link
Member Author

Ah, I read that as an issue in programmer friendliness, not performance. Also, I kind of disagree. We generally want to block the server when the client isn't ready. Otherwise, we'd have to buffer all content we aren't ready to handle (could be gigabytes). This is a special case because dropping a few progress updates isn't a problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants