]> git.ipfire.org Git - thirdparty/zstd.git/commitdiff
Nits 3127/head
authorElliot Gorokhovsky <embg@fb.com>
Thu, 12 May 2022 16:53:15 +0000 (12:53 -0400)
committerElliot Gorokhovsky <embg@fb.com>
Thu, 12 May 2022 16:53:15 +0000 (12:53 -0400)
lib/compress/zstd_double_fast.c
lib/compress/zstd_fast.c
lib/compress/zstd_lazy.c

index 698a97a802f317c48924ea0521d55d997705df17..d8412ef0b7e653115921cc712d85aa974e2bbc1a 100644 (file)
@@ -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;
index 3d3b7d9cb4f9a461180524cee10687ee3918f998..c7d48eb2414937cc80ed4db3cbec4d78ac660dd1 100644 (file)
@@ -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;
index 9814943d1a92ed1e8537521f995c18c96abc815d..912b59b9c606f0ea589130bc231d725bc7d57ee2 100644 (file)
@@ -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;