Converge sumtype (offset | repcode) numeric representation towards offBase #2965
Conversation
directly at ZSTD_storeSeq() interface. In the process, remove ZSTD_REP_MOVE. This makes it possible, in future commits, to update and effectively simplify the naming scheme to properly label the updated processing pipeline : offset | repcode => offBase => offCode + offBits
subtle dependency on sumtype numeric representation
|
ping |
|
ping 2 |
| @@ -23,13 +23,64 @@ | |||
| # endif | |||
| #endif | |||
|
|
|||
| /* ************************************************************ | |||
There was a problem hiding this comment.
Is this change intended? It isn't mentioned in the summary.
There was a problem hiding this comment.
Yes,
PLATFORM_POSIX_VERSION is used a bit below
but it's not defined anywhere,
so it's silently == 0 by default, always,
or with the right set of compiler flags, it generates a warning, or an error.
Hence, I define it here, using the same definition as in platform.h.
There was a problem hiding this comment.
Weird... I've been refreshing this page, but GitHub didn't show me this comment until now.
Can you also mention this in the PR summary please? Then the PR LGTM.
|
Can you please also mention what the first commit, "fixed minor compression difference in btlazy2" is fixing? It isn't clear what the problem is. And I didn't realize that this PR had a functional change until I looked at that commit title, I assumed it was all refactoring. |
The PR, as a whole, is "just" a refactoring. |
|
Can you add that to the PR summary please? |
done |
This completes #2962 , by ensuring that the numeric representation of the sumtype
(offset | repcode)used at the interface ofZSTD_storeSeq()(and elsewhere) is the same asoffBase, as stored in theseqStore.This skips one transient numeric representation, hopefully simplifying the pipeline.
It's a good opportunity to rename all these transformations to better reflect their position in the pipeline, aka :
offset | repcode=>offBase=>offCode+offBitsfollowing the model previously established for
matchLength.This PR does not impact the compressed output.
The first commit left a subtle minor difference in
btlazy2parser, resulting in rare minor differences in compressed output. This was later fixed in the last commit, so now there is no difference, even forbtlazy2.Also :
fixed
PLATFORM_POSIX_VERSIONwithinzstdseek_decompress.c.This macro was used there without being defined.
This triggered warning / errors on my workstation during tests.