fix(dictBuilder): keep dictItem slot index valid across the merge fixpoint

ZDICT_insertDictItem identifies the absorbed entry by its slot index while
iterating ZDICT_tryMerge to a fixpoint. ZDICT_tryMerge ends each successful
merge with a rank-improving insertion sort, which moves every entry between
the absorbing entry's old and new slot one slot back. When the caller's index
lies in that range it no longer denotes the absorbed entry, so the following
ZDICT_removeDictItem deletes an unrelated live segment and the absorbed entry
survives as a duplicate. Separately, ZDICT_removeDictItem shifts every entry
above the removed slot one slot forward, and newMerge is carried into the next
round of the fixpoint without that adjustment.

eltNbToSkip becomes a U32*. Each rank sort reports whether it displaced that
slot, and the fixpoint re-reads the slot after both the sort and the removal.

Reproduction: insert (pos,length,savings) = (1000,10,200), (340,20,100),
(300,20,90), (320,30,60). The fourth item bridges the second and third, the
cascading merge lifts the absorbing entry from slot 3 to slot 1, and the
segment at position 1000 -- which takes part in no merge and lies 700 bytes
from the nearest other interval -- is deleted.

Only ZDICT_trainFromBuffer_legacy and the CLI's --train-legacy reach this
code. ZDICT_trainFromBuffer routes to ZDICT_optimizeTrainFromBuffer_fastCover
and is unaffected. There is no public API change, no dictionary format change,
and no change to the ranking heuristic or the savings computation.

Signed-off-by: teerth sharma <teerths57@gmail.com>
This commit is contained in:
teerth sharma 2026-08-02 07:15:15 +05:30 committed by teerth sharma
parent 82d322c497
commit fc2b6a358b

View file

@ -356,10 +356,14 @@ static int isIncluded(const void* in, const void* container, size_t length)
/*! ZDICT_tryMerge() :
check if dictItem can be merged, do it if possible
@return : id of destination elt, 0 if not merged
note : *eltNbToSkipPtr designates an entry by its slot. The rank sorts below
can move that entry to the next higher slot; the value is updated
when they do, so the caller's index keeps designating the same entry.
*/
static U32 ZDICT_tryMerge(dictItem* table, dictItem elt, U32 eltNbToSkip, const void* buffer)
static U32 ZDICT_tryMerge(dictItem* table, dictItem elt, U32* eltNbToSkipPtr, const void* buffer)
{
const U32 tableSize = table->pos;
const U32 eltNbToSkip = *eltNbToSkipPtr;
const U32 eltEnd = elt.pos + elt.length;
const char* const buf = (const char*) buffer;
@ -375,9 +379,14 @@ static U32 ZDICT_tryMerge(dictItem* table, dictItem elt, U32 eltNbToSkip, const
table[u].savings += elt.length / 8; /* rough approx bonus */
elt = table[u];
/* sort : improve rank */
while ((u>1) && (table[u-1].savings < elt.savings))
table[u] = table[u-1], u--;
table[u] = elt;
{ U32 const startU = u;
while ((u>1) && (table[u-1].savings < elt.savings))
table[u] = table[u-1], u--;
table[u] = elt;
/* entries in [u, startU-1] each moved to the next higher slot */
if ((eltNbToSkip >= u) && (eltNbToSkip < startU))
*eltNbToSkipPtr = eltNbToSkip + 1;
}
return u;
} }
@ -395,9 +404,14 @@ static U32 ZDICT_tryMerge(dictItem* table, dictItem elt, U32 eltNbToSkip, const
}
/* sort : improve rank */
elt = table[u];
while ((u>1) && (table[u-1].savings < elt.savings))
table[u] = table[u-1], u--;
table[u] = elt;
{ U32 const startU = u;
while ((u>1) && (table[u-1].savings < elt.savings))
table[u] = table[u-1], u--;
table[u] = elt;
/* entries in [u, startU-1] each moved to the next higher slot */
if ((eltNbToSkip >= u) && (eltNbToSkip < startU))
*eltNbToSkipPtr = eltNbToSkip + 1;
}
return u;
}
@ -431,12 +445,23 @@ static void ZDICT_removeDictItem(dictItem* table, U32 id)
static void ZDICT_insertDictItem(dictItem* table, U32 maxSize, dictItem elt, const void* buffer)
{
/* merge if possible */
U32 mergeId = ZDICT_tryMerge(table, elt, 0, buffer);
U32 skipId = 0;
U32 mergeId = ZDICT_tryMerge(table, elt, &skipId, buffer);
if (mergeId) {
U32 newMerge = 1;
while (newMerge) {
newMerge = ZDICT_tryMerge(table, table[mergeId], mergeId, buffer);
if (newMerge) ZDICT_removeDictItem(table, mergeId);
/* table[absorbedId] is merged away, so it must be dropped.
* Both operations below can move it, and the slot is re-read
* after each one. */
U32 absorbedId = mergeId;
skipId = absorbedId;
newMerge = ZDICT_tryMerge(table, table[absorbedId], &skipId, buffer);
absorbedId = skipId; /* the rank sort may have moved it one slot up */
if (newMerge) {
ZDICT_removeDictItem(table, absorbedId);
/* the removal moved every higher slot down by one */
if (newMerge > absorbedId) newMerge--;
}
mergeId = newMerge;
}
return;