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.

Sign up for GitHub

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

patch-from speed optimization #3545

Merged
merged 7 commits into from
Mar 15, 2023
Merged

patch-from speed optimization #3545

Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f49395f
patch-from speed optimization: only load portion of dictionary into n…
daniellerozenblit Mar 10, 2023
9d8d9a4
test regression for x8 multiplier
daniellerozenblit Mar 10, 2023
300a59d
fix off-by-one error for bit shift bound
daniellerozenblit Mar 10, 2023
8d0a06a
restrict patchfrom speed optimization to strategy < ZSTD_btultra
daniellerozenblit Mar 13, 2023
f03b126
update results.csv
daniellerozenblit Mar 14, 2023
f0f518b
Merge branch 'dev' into patch-from-speed-optimization
daniellerozenblit Mar 14, 2023
4a5de4d
update regression test
daniellerozenblit Mar 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 17 additions & 8 deletions lib/compress/zstd_compress.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4669,7 +4669,8 @@ static size_t ZSTD_loadDictionaryContent(ZSTD_matchState_t* ms,
ip = iend - maxDictSize;
src = ip;
srcSize = maxDictSize;
} }
}
}

if (srcSize > ZSTD_CHUNKSIZE_MAX) {
/* We must have cleared our windows when our source is this large. */
Expand All @@ -4678,22 +4679,30 @@ static size_t ZSTD_loadDictionaryContent(ZSTD_matchState_t* ms,
}

DEBUGLOG(4, "ZSTD_loadDictionaryContent(): useRowMatchFinder=%d", (int)params->useRowMatchFinder);
ZSTD_window_update(&ms->window, src, srcSize, /* forceNonContiguous */ 0);
ms->loadedDictEnd = params->forceWindow ? 0 : (U32)(iend - ms->window.base);
ms->forceNonContiguous = params->deterministicRefPrefix;

if (loadLdmDict) {
if (loadLdmDict) { /* Load the entire dict into LDM matchfinders. */
ZSTD_window_update(&ls->window, src, srcSize, /* forceNonContiguous */ 0);
ls->loadedDictEnd = params->forceWindow ? 0 : (U32)(iend - ls->window.base);
ZSTD_ldm_fillHashTable(ls, ip, iend, &params->ldmParams);
}

/* If the dict is larger than we can reasonably index in our tables, only load the suffix. */
{ U32 maxDictSize = 8U * (1U << MIN(MAX(params->cParams.hashLog, params->cParams.chainLog), 29));
Copy link
Contributor

@Cyan4973 Cyan4973 Mar 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This is a bit too much.
8 * (1<<29) is equivalent to 1<<32, so this will not end well on 32-bit values.
1<<31 (2 GB) shall be the max, it is (currently) our limit window size anyway.
8 * MIN(MAX(),28) should do the trick.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, silly mistake! Thanks for catching this.

if (srcSize > maxDictSize) {
ip = iend - maxDictSize;
src = ip;
srcSize = maxDictSize;
}
}

ZSTD_window_update(&ms->window, src, srcSize, /* forceNonContiguous */ 0);
ms->loadedDictEnd = params->forceWindow ? 0 : (U32)(iend - ms->window.base);
ms->forceNonContiguous = params->deterministicRefPrefix;

if (srcSize <= HASH_READ_SIZE) return 0;

ZSTD_overflowCorrectIfNeeded(ms, ws, params, ip, iend);

if (loadLdmDict)
ZSTD_ldm_fillHashTable(ls, ip, iend, &params->ldmParams);

switch(params->cParams.strategy)
{
case ZSTD_fast:
Expand Down