> On Nov 17, 2021, at 11:52 PM, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote: > > Hi Nick, > > On Wed, Nov 17, 2021 at 9:08 PM Nick Terrell <nickrterrell@xxxxxxxxx> wrote: >> `zstd_opt.c` contains the match finder for the highest compression >> levels. These levels are already very slow, and are unlikely to be used >> in the kernel. If they are used, they shouldn't be used in latency >> sensitive workloads, so slowing them down shouldn't be a big deal. >> >> This saves 188 KB of the 288 KB regression reported by Geert Uytterhoeven [0]. >> I've also opened an issue upstream [1] so that we can properly tackle >> the code size issue in `zstd_opt.c` for all users, and can hopefully >> remove this hack in the next zstd version we import. >> >> Bloat-o-meter output on x86-64: >> >> ``` >>> ../scripts/bloat-o-meter vmlinux.old vmlinux >> add/remove: 6/5 grow/shrink: 1/9 up/down: 16673/-209939 (-193266) >> Function old new delta >> ZSTD_compressBlock_opt_generic.constprop - 7559 +7559 >> ZSTD_insertBtAndGetAllMatches - 6304 +6304 >> ZSTD_insertBt1 - 1731 +1731 >> ZSTD_storeSeq - 693 +693 >> ZSTD_BtGetAllMatches - 255 +255 >> ZSTD_updateRep - 128 +128 >> ZSTD_updateTree 96 99 +3 >> ZSTD_insertAndFindFirstIndexHash3 81 - -81 >> ZSTD_setBasePrices.constprop 98 - -98 >> ZSTD_litLengthPrice.constprop 138 - -138 >> ZSTD_count 362 181 -181 >> ZSTD_count_2segments 1407 938 -469 >> ZSTD_insertBt1.constprop 2689 - -2689 >> ZSTD_compressBlock_btultra2 19990 423 -19567 >> ZSTD_compressBlock_btultra 19633 15 -19618 >> ZSTD_initStats_ultra 19825 - -19825 >> ZSTD_compressBlock_btopt 20374 12 -20362 >> ZSTD_compressBlock_btopt_extDict 29984 12 -29972 >> ZSTD_compressBlock_btultra_extDict 30718 15 -30703 >> ZSTD_compressBlock_btopt_dictMatchState 32689 12 -32677 >> ZSTD_compressBlock_btultra_dictMatchState 33574 15 -33559 >> Total: Before=6611828, After=6418562, chg -2.92% >> ``` >> >> [0] https://lkml.org/lkml/2021/11/14/189 >> [1] https://github.com/facebook/zstd/issues/2862 >> >> Reported-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> >> Signed-off-by: Nick Terrell <terrelln@xxxxxx> > > Thanks for your patch! > > Impact on lib/zstd/zstd_compress.ko for atari_defconfig: > > add/remove: 5/4 grow/shrink: 1/9 up/down: 15392/-167214 (-151822) > > Nice! > > Tested-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> > >> --- a/lib/zstd/compress/zstd_opt.c >> +++ b/lib/zstd/compress/zstd_opt.c >> @@ -894,7 +906,7 @@ static void ZSTD_optLdm_processMatchCandidate(ZSTD_optLdm_t* optLdm, ZSTD_match_ >> */ >> U32 posOvershoot = currPosInBlock - optLdm->endPosInBlock; >> ZSTD_optLdm_skipRawSeqStoreBytes(&optLdm->seqStore, posOvershoot); >> - } >> + } >> ZSTD_opt_getNextMatchAndUpdateSeqStore(optLdm, currPosInBlock, remainingBytes); >> } >> ZSTD_optLdm_maybeAddMatch(matches, nbMatches, optLdm, currPosInBlock); > > This change is unrelated. With that removed: > Reviewed-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> Thanks for the review! I will remove this change before submitting the PR. Unless you have a strong preference, I won’t put up another version of the patch set with just this change. Best, Nick Terrell > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
