In Compressor.AddWord, SamplingFactor > 1 does not sample every Nth superstring as intended — it stops sampling entirely after the first one:
l := 2*len(word) + 2
if len(c.superstring)+l > superstringLimit {
if c.superstringCount%c.SamplingFactor == 0 {
c.superstrings <- c.superstring
}
c.superstringCount++
c.superstring = superStringsPool.Get().([]byte)[:0]
}
if c.superstringCount%c.SamplingFactor == 0 {
for _, a := range word { ... } // append
}Once superstringCount%SamplingFactor != 0, words are no longer appended, so the buffer never refills, the overflow branch never runs, and superstringCount never advances again. Net effect: with SamplingFactor > 1 the pattern dictionary is built from the first 16MB of the file only.
seg.DefaultCfg (and the history/II compressor configs) use SamplingFactor: 4, so all such files have been building dictionaries from their first 16MB. Empirically this often works (and on sorted storage keys it even produced a slightly smaller file than full analysis), but it is not the documented/intended behavior, and a first-16MB sample is biased for sorted inputs.
Fixing the loop (e.g. tracking skipped bytes so superstringCount advances through skip phases) changes dictionary quality and collation cost for every SamplingFactor>1 user, so it should come with before/after measurements of segment sizes and collation/merge times — found while working on #21625, where the domain SamplingFactor 1→4 change was dropped for exactly this reason.
In
Compressor.AddWord,SamplingFactor > 1does not sample every Nth superstring as intended — it stops sampling entirely after the first one:Once
superstringCount%SamplingFactor != 0, words are no longer appended, so the buffer never refills, the overflow branch never runs, andsuperstringCountnever advances again. Net effect: withSamplingFactor > 1the pattern dictionary is built from the first 16MB of the file only.seg.DefaultCfg(and the history/II compressor configs) useSamplingFactor: 4, so all such files have been building dictionaries from their first 16MB. Empirically this often works (and on sorted storage keys it even produced a slightly smaller file than full analysis), but it is not the documented/intended behavior, and a first-16MB sample is biased for sorted inputs.Fixing the loop (e.g. tracking skipped bytes so
superstringCountadvances through skip phases) changes dictionary quality and collation cost for everySamplingFactor>1user, so it should come with before/after measurements of segment sizes and collation/merge times — found while working on #21625, where the domainSamplingFactor1→4 change was dropped for exactly this reason.