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

Improves compression ratio for small windowLog #1624

Merged
merged 7 commits into from
Jun 15, 2019
Merged

Improves compression ratio for small windowLog #1624

merged 7 commits into from
Jun 15, 2019

Conversation

Cyan4973
Copy link
Contributor

@Cyan4973 Cyan4973 commented Jun 1, 2019

To ensure that all detected matches remain within window size,
the current strategy is to preemptively invalidate all indexes
that could end up being too far while compressing next block.

This is good for speed, as checking if a match is within distance is now a trivial comparison.

This roughly invalidates one block worth of data, which is about 128 KB.
It's then progressively restored as compression progress through the block. But temporarily, the window size has been truncated by up to 128 KB.
This is bad for compression ratio, but the impact is low, as long as the window size is large, relative to the block size.

As the window size is reduced, this is no longer the case.
Especially when wlog <= 17, the window size is now equal to the block size. Hence, this strategy invalidates all history before each block (assuming each block is full), making each block effectively independent. This has a big impact on compression ratio.

In this patch, the logic is changed so that each strategy selects how to best deal with maximum offset distance, aka, the window size.

Logic for fast and double_fast is unchanged, as these single-probe strategies want to spend very little time per attempt.

Logic for all strategies >= greedy is modified, to accepts all candidates, up to maximum window size. This makes them more effective in combination with small window sizes.

Example on enwik7 at level 19 :

windowLog ratio on dev this patch improvement
23 3.577 3.577 0.02%
22 3.536 3.538 0.06%
21 3.462 3.467 0.14%
20 3.364 3.377 0.39%
19 3.244 3.272 0.86%
18 3.110 3.166 1.80%
17 2.843 3.057 7.53%
16 2.724 2.943 8.04%
15 2.594 2.822 8.79%
14 2.456 2.686 9.36%
13 2.312 2.523 9.13%
12 2.162 2.361 9.20%
11 2.003 2.182 8.94%

As expected, improvement increases as window size decreases. It's negligible at wlog = 23, then roughly doubles with each reduction of wlog.
On reaching wlog = 17, there is a visible bump. After that, the gain stabilize around +8-9%, which is quite significant.

noticeably improves compression ratio
when window size is small (< 18).

enwik7	level 19

windowLog	`dev`	`smallwlog`	improvement
23	3.577	3.577	0.02%
22	3.536	3.538	0.06%
21	3.462	3.467	0.14%
20	3.364	3.377	0.39%
19	3.244	3.272	0.86%
18	3.110	3.166	1.80%
17	2.843	3.057	7.53%
16	2.724	2.943	8.04%
15	2.594	2.822	8.79%
14	2.456	2.686	9.36%
13	2.312	2.523	9.13%
12	2.162	2.361	9.20%
11	2.003	2.182	8.94%
fast mode does the same thing as before :
it pre-emptively invalidates any index that could lead to offset > maxDistance.
It's supposed to help speed.

But this logic is performed inside zstd_fast,
so that other strategies can select a different behavior.
apparently not guaranteed on all platforms,
replaced by UINT_MAX.
Copy link
Contributor

@felixhandte felixhandte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see ZSTD_window_enforceMaxDist() is also used in zstd_ldm.c. Do you plan to extend this modification to that compressor? I guess the large-window nature of LDM makes this probably not valuable for ratio purposes. But it still might be nice to bring it into alignment so we can remove ZSTD_window_enforceMaxDist() entirely?

The other thing I'm wondering about is the forceWindow option. Does this remove the incompatibility between forceWindow and attachDict? My sense is that it does. I guess we can merge this and I can then experiment with removing that exclusion.

@Cyan4973
Copy link
Contributor Author

Cyan4973 commented Jun 3, 2019

Indeed, I intentionally left ZSTD_window_enforceMaxDist () in order to not touch zstd_ldm.c.
This part of the code base is less familiar to me, therefore I can't guarantee a quick and safe fix without side-effect, jeopardizing the main objective of this patch.

I think it can still be achieved as an improvement over this patch.
It seems that ZSTD_window_enforceMaxDist() is now only used in zstd_ldm.c, so it could be moved there to begin with.

Does this remove the incompatibility between forceWindow and attachDict?

I'm not sure, this consequence does not look obvious to me.
This wasn't an objective, so if it's achieved, it's by accident.

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

Successfully merging this pull request may close these issues.

3 participants