From 3620a0a56589c952d248dfccdc96a86b948e9432 Mon Sep 17 00:00:00 2001 From: Elliot Gorokhovsky Date: Thu, 12 May 2022 12:53:15 -0400 Subject: [PATCH] Nits --- lib/compress/zstd_double_fast.c | 6 +++--- lib/compress/zstd_fast.c | 23 +++++++++++++++++------ lib/compress/zstd_lazy.c | 6 +++--- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/lib/compress/zstd_double_fast.c b/lib/compress/zstd_double_fast.c index 698a97a80..d8412ef0b 100644 --- a/lib/compress/zstd_double_fast.c +++ b/lib/compress/zstd_double_fast.c @@ -175,9 +175,9 @@ size_t ZSTD_compressBlock_doubleFast_noDict_generic( } while (ip1 <= ilimit); _cleanup: - /* If offset_1 started invalid (offsetSaved1 > 0) and became valid (offset_1 > 0), - * rotate saved offsets. */ - offsetSaved2 = ((offsetSaved1 > 0) & (offset_1 > 0)) ? offsetSaved1 : offsetSaved2; + /* If offset_1 started invalid (offsetSaved1 != 0) and became valid (offset_1 != 0), + * rotate saved offsets. See comment in ZSTD_compressBlock_fast_noDict for more context. */ + offsetSaved2 = ((offsetSaved1 != 0) && (offset_1 != 0)) ? offsetSaved1 : offsetSaved2; /* save reps for next block */ rep[0] = offset_1 ? offset_1 : offsetSaved1; diff --git a/lib/compress/zstd_fast.c b/lib/compress/zstd_fast.c index 3d3b7d9cb..c7d48eb24 100644 --- a/lib/compress/zstd_fast.c +++ b/lib/compress/zstd_fast.c @@ -254,9 +254,20 @@ _cleanup: * However, it seems to be a meaningful performance hit to try to search * them. So let's not. */ - /* If rep_offset1 started invalid (offsetSaved1 > 0) and became valid (rep_offset1 > 0), - * rotate saved offsets. */ - offsetSaved2 = ((offsetSaved1 > 0) & (rep_offset1 > 0)) ? offsetSaved1 : offsetSaved2; + /* When the repcodes are outside of the prefix, we set them to zero before the loop. + * When the offsets are still zero, we need to restore them after the block to have a correct + * repcode history. If only one offset was invalid, it is easy. The tricky case is when both + * offsets were invalid. We need to figure out which offset to refill with. + * - If both offsets are zero they are in the same order. + * - If both offsets are non-zero, we won't restore the offsets from `offsetSaved[12]`. + * - If only one is zero, we need to decide which offset to restore. + * - If rep_offset1 is non-zero, then rep_offset2 must be offsetSaved1. + * - It is impossible for rep_offset2 to be non-zero. + * + * So if rep_offset1 started invalid (offsetSaved1 != 0) and became valid (rep_offset1 != 0), then + * set rep[0] = rep_offset1 and rep[1] = offsetSaved1. + */ + offsetSaved2 = ((offsetSaved1 != 0) && (rep_offset1 != 0)) ? offsetSaved1 : offsetSaved2; /* save reps for next block */ rep[0] = rep_offset1 ? rep_offset1 : offsetSaved1; @@ -762,9 +773,9 @@ _cleanup: * However, it seems to be a meaningful performance hit to try to search * them. So let's not. */ - /* If offset_1 started invalid (offsetSaved1 > 0) and became valid (offset_1 > 0), - * rotate saved offsets. */ - offsetSaved2 = ((offsetSaved1 > 0) & (offset_1 > 0)) ? offsetSaved1 : offsetSaved2; + /* If offset_1 started invalid (offsetSaved1 != 0) and became valid (offset_1 != 0), + * rotate saved offsets. See comment in ZSTD_compressBlock_fast_noDict for more context. */ + offsetSaved2 = ((offsetSaved1 != 0) && (offset_1 != 0)) ? offsetSaved1 : offsetSaved2; /* save reps for next block */ rep[0] = offset_1 ? offset_1 : offsetSaved1; diff --git a/lib/compress/zstd_lazy.c b/lib/compress/zstd_lazy.c index 9814943d1..912b59b9c 100644 --- a/lib/compress/zstd_lazy.c +++ b/lib/compress/zstd_lazy.c @@ -1682,9 +1682,9 @@ _storeSequence: continue; /* faster when present ... (?) */ } } } - /* If offset_1 started invalid (offsetSaved1 > 0) and became valid (offset_1 > 0), - * rotate saved offsets. */ - offsetSaved2 = ((offsetSaved1 > 0) & (offset_1 > 0)) ? offsetSaved1 : offsetSaved2; + /* If offset_1 started invalid (offsetSaved1 != 0) and became valid (offset_1 != 0), + * rotate saved offsets. See comment in ZSTD_compressBlock_fast_noDict for more context. */ + offsetSaved2 = ((offsetSaved1 != 0) && (offset_1 != 0)) ? offsetSaved1 : offsetSaved2; /* save reps for next block */ rep[0] = offset_1 ? offset_1 : offsetSaved1; -- 2.47.2