zstd_opt: changed cost formula
There was a flaw in the formula which compared literal cost with match cost : at a given position, a non-null literal suite is going to be part of next sequence, while if position ends a previous match, to immediately start another match, next sequence will have a litlength of zero. A litlength of zero has a non-null cost. It follows that literals cost should be compared to match cost + litlength==0. Not doing so gave a structural advantage to matches, which would be selected more often. I believe that's what led to the creation of the strange heuristic which added a complex cost to matches. The heuristic was actually compensating. It was probably created through multiple trials, settling for best outcome on a given scenario (I suspect silesia.tar). The problem with this heuristic is that it's hard to understand, and unfortunately, any future change in the parser would impact the way it should be calculated and its effects. The "proper" formula makes it possible to remove this heuristic. Now, the problem is : in a head to head comparison, it's sometimes better, sometimes worse. Note that all differences are small (< 0.01 ratio). In general, the newer formula is better for smaller files (for example, calgary.tar and enwik7). I suspect that's because starting statistics are pretty poor (another area of improvement). However, for silesia.tar specifically, it's worse at level 22 (while being better at level 17, so even compression level has an impact ...). It's a pity that zstd -22 gets worse on silesia.tar. That being said, I like that the new code gets rid of strange variables, which were introducing complexity for any future evolution (faster variants being in mind). Therefore, in spite of this detrimental side effect, I tend to be in favor of it.
This commit is contained in:
@@ -64,7 +64,7 @@ typedef struct {
|
||||
} ZSTD_match_t;
|
||||
|
||||
typedef struct {
|
||||
U32 price;
|
||||
int price;
|
||||
U32 off;
|
||||
U32 mlen;
|
||||
U32 litlen;
|
||||
@@ -83,14 +83,12 @@ typedef struct {
|
||||
U32 litSum; /* nb of literals */
|
||||
U32 litLengthSum; /* nb of litLength codes */
|
||||
U32 matchLengthSum; /* nb of matchLength codes */
|
||||
U32 matchSum; /* a strange argument used in calculating `factor` */
|
||||
U32 offCodeSum; /* nb of offset codes */
|
||||
/* begin updated by ZSTD_setLog2Prices */
|
||||
U32 log2litSum; /* pow2 to compare log2(litfreq) to */
|
||||
U32 log2litLengthSum; /* pow2 to compare log2(llfreq) to */
|
||||
U32 log2matchLengthSum; /* pow2 to compare log2(mlfreq) to */
|
||||
U32 log2offCodeSum; /* pow2 to compare log2(offreq) to */
|
||||
U32 factor; /* fixed cost added when calculating ZSTD_getPrice() (but why ? seems to favor less sequences) */
|
||||
/* end : updated by ZSTD_setLog2Prices */
|
||||
U32 staticPrices; /* prices follow a pre-defined cost structure, statistics are irrelevant */
|
||||
} optState_t;
|
||||
|
||||
Reference in New Issue
Block a user