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

Chunk buffer size is inconsistent with std recommendation #193

Open
HeroicKatora opened this issue Mar 31, 2020 · 2 comments
Open

Chunk buffer size is inconsistent with std recommendation #193

HeroicKatora opened this issue Mar 31, 2020 · 2 comments

Comments

@HeroicKatora
Copy link
Member

NB: stdlib recommends 8k for the size of its own BufWriter.

Originally posted by @nabijaczleweli in #190

The internally used ChunkWriter has a similar purpose. Can we reuse the standard buffer here or refer to the standard definition, or change the buffer size? What are the benefits and motivation of that recommendation?

@nabijaczleweli
Copy link
Member

That constant moved there in rust-lang/rust@6d54cd4, and comments reference rust-lang/rust@8128817:

64K was picked for symmetry with libuv, which we no longer use.
64K is way larger than the default size of any other language that I can find. C, C++, and Java default to 8K, and Go defaults to 4K. There have been a variety of issues filed relating to this such as #31885.

The referenced libuv diff (https://github.com/rust-lang/rust/pull/9091/files#diff-b131eeef531ad098b32f49695a031008R62) says

// libuv recommends 64k buffers to maximize throughput
// https://groups.google.com/forum/#!topic/libuv/oQO1HJAIDdA
static DEFAULT_CAPACITY: uint = 64 * 1024;

Relevant mail:

libuv asks for 64K because that lets us (nearly always) drain the kernel receive buffer with a single syscall.
On a tangential note, I've played around with querying the number of pending bytes but the extra syscalls degrade performance by a substantial margin, 25-40% on most benchmarks.

@nabijaczleweli
Copy link
Member

I looked at the Writer/ChunkWriter/ChunkOutput implementation and I'm pretty sure one explicitly-buffering sink is required (ChunkWriter), so I don't think that can be replaced with an stdlib BufWriter. Well, could be replaced with having a buffer in Writer itself and implementing Write on that, but that's equivalent.

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