Can we save huge amounts of directory bandwith with Z_NO_FLUSH?

It seems our default argument when compressing with zlib is Z_SYNC_FLUSH. That's not ideal, though: in theory we would get better compression by using Z_NO_FLUSH. But, how much better? And would that work with our code, or would we need other changes?

We should at least investigate this in 0.2.5, though if the fix is nontrivial, it can wait for 0.2.6.

changed milestone to %Tor: 0.2.6.x-final

added 025-backport 026-triaged-0 026-triaged-1 component::core tor/tor milestone::Tor: 0.2.6.x-final priority::medium resolution::fixed status::closed tor-relay type::enhancement zlib labels

Totally untested patch in branch "bug11787" in my public repository

Trac:
Status: new to needs_review

I still don't know if the patch works. But if it does, I expect it to save something 7% for server descriptors, and who-knows-how-much for microdescs. (This is based on downloading all.z from turtles, uncompressing it, and recompressing it with gzip -9)

(The actual number of bytes saved will depend on the number of compressed objects and their actual formats)

Trac:
zlibtest.py

I've attached a fun script to try compressing different numbers of descriptors and microdescriptors with and without Z_SYNC_FLUSH.

Preliminary results are that we should expect to save 5-7% when sending 10-40 server descriptors, and between 12-17% when sending 10-100 microdescriptors. Worth doing IMO.

(This fix probably needs the #11648 (moved) fix as well, or i would expect it to sometimes cause us to truncate streams and omit many descriptors)

Replying to nickm:

Totally untested patch in branch "bug11787" in my public repository

Not sure and maybe the patch is only for benchmarking / testing out things, but wouldn't the _de_compression part in torgzip.c also need changing?

302     switch (inflate(stream, complete_only ? Z_FINISH : Z_SYNC_FLUSH))

and

477     err = inflate(&state->stream, finish ? Z_FINISH : Z_SYNC_FLUSH);

or am I mistaken?

I think Z_SYNC_FLUSH is safe on inflate. After all, there is only one correct output for inflating any compressed zlib stream. (Anybody want to look at the zlib source to see what's up?)

Moving to 0.2.6 with possible backport, since the effects of breaking this code would not be fun to try to solve late in a release series.

Trac:
Milestone: Tor: 0.2.5.x-final to Tor: 0.2.6.x-final
Keywords: tor-relay zlib deleted, zlib, tor-relay, 025-backport added

Trac:
Keywords: 025-backport deleted, 025-backport 026-triaged added

Trac:
Keywords: 025-backport 026-triaged deleted, 026-triaged-0, 025-backport added

Trac:
Keywords: 026-triaged-0 deleted, 026-triaged-0 026-triaged-1 added

Merged to master, marking for possible backport. Seems to work ok in testing.

Trac:
Milestone: Tor: 0.2.6.x-final to Tor: 0.2.5.x-final

On first glance, this looks like exactly the sort of feature that doesn't need a backport. (Let's make 0.2.6 the new stable and leave 0.2.5 behind.)

ack

Trac:
Status: needs_review to closed
Resolution: N/A to fixed
Type: defect to enhancement
Milestone: Tor: 0.2.5.x-final to Tor: 0.2.6.x-final

closed

mentioned in issue #11790 (moved)

mentioned in issue #11791 (moved)

mentioned in issue #16801 (moved)

moved to tpo/core/tor#11787 (closed)

mentioned in issue tpo/core/tor#11790 (closed)

mentioned in issue tpo/core/tor#11791 (closed)

mentioned in issue tpo/core/tor#16801 (closed)