Skip to content

Commit a5bf3dd

Browse files
danlark1danlark1
committed
Add ZSTD_HASH_USE_CRC32C to use an optimized hash
At Google we found 3-5% improvement for densely compressed data when using CRC32C and low bits of its hash. Most uplift should be seen with at least `-msse4.2` flag for x86-64 and march=armv8-a+crc extension (from Arm8.3 CRC is mandatory). In this patch we added a macro, fixed implementation details which relied on shifts and added CI tests with different configurations. We also added a crc32c regression test to compare sizes and verify it's not regressing the values too much.
1 parent d7ee320 commit a5bf3dd

File tree

11 files changed

+1838
-71
lines changed

11 files changed

+1838
-71
lines changed

‎.github/workflows/commit.yml‎

Lines changed: 55 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,16 @@ jobs:
5151
run: make gnu99build && make clean
5252
- name: ppc64 build
5353
run: make ppc64build V=1 && make clean
54+
- name: ppc64CRC build
55+
run: make ppc64buildCRC V=1 && make clean
5456
- name: ppc build
5557
run: make ppcbuild V=1 && make clean
5658
- name: arm build
5759
run: make armbuild V=1 && make clean
5860
- name: aarch64 build
5961
run: make aarch64build V=1 && make clean
62+
- name: aarch64CRC build
63+
run: make aarch64buildCRC V=1 && make clean
6064
- name: test-legacy
6165
run: make -C tests test-legacy V=1 && make clean
6266
- name: test-longmatch
@@ -65,40 +69,81 @@ jobs:
6569
run: make -C lib libzstd-nomt V=1 && make clean
6670

6771
regression-test:
68-
runs-on: ubuntu-latest
72+
name: regression-${{ matrix.config_name }}
73+
runs-on: ${{ matrix.os }}
74+
strategy:
75+
fail-fast: false
76+
matrix:
77+
include:
78+
- config_name: "default"
79+
os: ubuntu-latest
80+
baseline: "results.csv"
81+
cflags: "-O3"
82+
83+
- config_name: "x86-crc"
84+
os: ubuntu-latest
85+
baseline: "results_crc32c.csv"
86+
cflags: "-O3 -DZSTD_HASH_USE_CRC32C"
87+
88+
- config_name: "x86-sse4.2-crc"
89+
os: ubuntu-latest
90+
baseline: "results_crc32c.csv"
91+
cflags: "-O3 -DZSTD_HASH_USE_CRC32C -msse4.2"
92+
93+
- config_name: "arm-v8.2-crc"
94+
os: ubuntu-24.04-arm
95+
baseline: "results_crc32c.csv"
96+
cflags: "-O3 -DZSTD_HASH_USE_CRC32C -march=armv8.2-a+crc+crypto"
97+
6998
services:
7099
docker:
71100
image: fbopensource/zstd-circleci-primary:0.0.1
72101
options: --entrypoint /bin/bash
102+
73103
env:
74104
CIRCLE_ARTIFACTS: "/tmp/circleci-artifacts"
105+
CFLAGS: ${{ matrix.cflags }}
106+
75107
steps:
76108
- uses: actions/checkout@v6.0.2
109+
77110
- name: restore_cache
78111
uses: actions/cache@v5
79112
with:
80-
key: regression-cache-{{ checksum "tests/regression/data.c" }}-v0
113+
# Use matrix config_name to keep cache keys unique per architecture/config
114+
key: regression-cache-${{ matrix.config_name }}-${{ checksum "tests/regression/data.c" }}-v0
81115
path: tests/regression/cache
82-
restore-keys: regression-cache-{{ checksum "tests/regression/data.c" }}-v0
116+
83117
- name: Install Dependencies
84118
run: |
85119
sudo apt-get update
86120
sudo apt-get install libcurl4-gnutls-dev
121+
87122
- name: Regression Test
88123
run: |
124+
# Build zstd with the matrix-specific CFLAGS
89125
make -C programs zstd
90126
make -C tests/regression test
127+
91128
mkdir -p $CIRCLE_ARTIFACTS
129+
130+
# Run the regression test tool
92131
./tests/regression/test \
93132
--cache tests/regression/cache \
94133
--output $CIRCLE_ARTIFACTS/results.csv \
95134
--zstd programs/zstd
96-
echo "NOTE: The new results.csv is uploaded as an artifact to this job"
97-
echo " If this fails, go to the Artifacts pane in CircleCI, "
98-
echo " download /tmp/circleci-artifacts/results.csv, and if they "
99-
echo " are still good, copy it into the repo and commit it."
100-
echo "> diff tests/regression/results.csv $CIRCLE_ARTIFACTS/results.csv"
101-
diff tests/regression/results.csv $CIRCLE_ARTIFACTS/results.csv
135+
136+
echo "Comparing against baseline: ${{ matrix.baseline }}"
137+
# Check if the baseline exists (to avoid failing on the first run of a new config)
138+
if [ -f "tests/regression/${{ matrix.baseline }}" ]; then
139+
diff tests/regression/${{ matrix.baseline }} $CIRCLE_ARTIFACTS/results.csv
140+
else
141+
echo "Warning: Baseline tests/regression/${{ matrix.baseline }} not found."
142+
echo "If this is a new configuration, please commit the generated results.csv as ${{ matrix.baseline }}."
143+
fi
144+
102145
- uses: actions/upload-artifact@v7.0.0
103146
with:
104-
path: "/tmp/circleci-artifacts"
147+
# Artifact name includes config for easy identification
148+
name: results-${{ matrix.config_name }}
149+
path: "$CIRCLE_ARTIFACTS/results.csv"

‎.github/workflows/dev-long-tests.yml‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,13 @@ jobs:
4242
- name: make test on macos
4343
run: make test
4444

45+
make-test-macos-crc32:
46+
runs-on: macos-latest
47+
steps:
48+
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # tag=v6.0.2
49+
- name: make test on macos with CRC32C
50+
run: CFLAGS="-DZSTD_HASH_USE_CRC32C" make test
51+
4552
# lasts ~10mn
4653
make-test-32bit:
4754
runs-on: ubuntu-latest
@@ -74,6 +81,21 @@ jobs:
7481
- name: no intrinsics fuzztest
7582
run: MOREFLAGS="-DZSTD_NO_INTRINSICS" make -C tests fuzztest
7683

84+
# lasts ~9mn
85+
crc-sse42-fuzztest:
86+
runs-on: ubuntu-latest
87+
steps:
88+
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # tag=v6.0.2
89+
- name: CRC32C fuzztest with sse4.2
90+
run: MOREFLAGS="-DZSTD_HASH_USE_CRC32C -msse4.2" make -C tests fuzztest
91+
92+
crc-nosse42-fuzztest:
93+
runs-on: ubuntu-latest
94+
steps:
95+
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # tag=v6.0.2
96+
- name: CRC32C fuzztest without sse4.2
97+
run: MOREFLAGS="-DZSTD_HASH_USE_CRC32C" make -C tests fuzztest
98+
7799
# lasts ~8mn
78100
tsan-zstreamtest:
79101
runs-on: ubuntu-latest
@@ -209,6 +231,13 @@ jobs:
209231
- name: clang + ASan + UBSan + Regression Test
210232
run: CC=clang make -j uasanregressiontest
211233

234+
clang-asan-ubsan-regressionCRC:
235+
runs-on: ubuntu-latest
236+
steps:
237+
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # tag=v6.0.2
238+
- name: clang + ASan + UBSan + Regression Test + CRC hash
239+
run: CC=clang make -j uasanregressiontestCRC
240+
212241
msan-regression:
213242
runs-on: ubuntu-latest
214243
steps:

‎.github/workflows/dev-short-tests.yml‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,16 @@ jobs:
153153
make libc6install
154154
CFLAGS="-Werror -mavx2" make -j all
155155
156+
gcc-make-all-avx2-crc:
157+
runs-on: ubuntu-latest
158+
steps:
159+
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # tag=v6.0.2
160+
- name: Make all, with AVX2 and CRC hash
161+
run: |
162+
sudo apt-get -qqq update
163+
make libc6install
164+
CFLAGS="-Werror -mavx2 -DZSTD_HASH_USE_CRC32C" make -j all
165+
156166
gcc-make-all-32bit:
157167
runs-on: ubuntu-latest
158168
steps:
@@ -173,6 +183,15 @@ jobs:
173183
make libc6install
174184
CPPFLAGS="-DSTATIC_BMI2=1" CFLAGS="-Werror -m32 -mavx2 -mbmi2" make -j all32
175185
186+
gcc-make-all-32bit-avx2-crc:
187+
runs-on: ubuntu-latest
188+
steps:
189+
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # tag=v6.0.2
190+
- name: Make all, 32bit + AVX2 mode and CRC hash
191+
run: |
192+
sudo apt-get -qqq update
193+
make libc6install
194+
CPPFLAGS="-DSTATIC_BMI2=1" CFLAGS="-Werror -m32 -mavx2 -mbmi2 -DZSTD_HASH_USE_CRC32C" make -j all32
176195
177196
gcc-8-make:
178197
runs-on: ubuntu-latest
@@ -434,8 +453,8 @@ jobs:
434453
make clean
435454
LDFLAGS="-static" CC=$XCC QEMU_SYS=$XEMU make -j check
436455
LDFLAGS="-static" CC=$XCC QEMU_SYS=$XEMU make -j -C tests test-cli-tests
437-
CFLAGS="-O3 -march=armv8.2-a+sve2" LDFLAGS="-static" CC=$XCC QEMU_SYS=$XEMU make -j check
438-
CFLAGS="-O3 -march=armv8.2-a+sve2" LDFLAGS="-static" CC=$XCC QEMU_SYS=$XEMU make -j -C tests test-cli-tests
456+
CFLAGS="-O3 -march=armv8.2-a+sve2+crc -DZSTD_HASH_USE_CRC32C" LDFLAGS="-static" CC=$XCC QEMU_SYS=$XEMU make -j check
457+
CFLAGS="-O3 -march=armv8.2-a+sve2+crc -DZSTD_HASH_USE_CRC32C" LDFLAGS="-static" CC=$XCC QEMU_SYS=$XEMU make -j -C tests test-cli-tests
439458
# This test is only compatible with standard libraries that support BTI (Branch Target Identification).
440459
# Unfortunately, the standard library provided on Ubuntu 24.04 does not have this feature enabled.
441460
# make clean

‎Makefile‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ clangbuild-darwin-fat: clean
205205
mv programs/zstd programs/zstd_x64
206206
lipo -create programs/zstd_x64 programs/zstd_arm64 -output programs/zstd
207207

208-
.PHONY: gcc5build gcc6build gcc7build clangbuild m32build armbuild aarch64build ppcbuild ppc64build
208+
.PHONY: gcc5build gcc6build gcc7build clangbuild m32build armbuild aarch64build aarch64buildCRC ppcbuild ppc64build ppc64buildCRC
209209
gcc5build: clean
210210
gcc-5 -v
211211
CC=gcc-5 $(MAKE) all MOREFLAGS="-Werror $(MOREFLAGS)"
@@ -232,12 +232,18 @@ armbuild: clean
232232
aarch64build: clean
233233
CC=aarch64-linux-gnu-gcc CFLAGS="-Werror -O0" $(MAKE) allzstd
234234

235+
aarch64buildCRC: clean
236+
CC=aarch64-linux-gnu-gcc CFLAGS="-Werror -O0 -DZSTD_HASH_USE_CRC32C -march=armv8.2-a+crc+crypto" $(MAKE) allzstd
237+
235238
ppcbuild: clean
236239
CC=powerpc-linux-gnu-gcc CFLAGS="-m32 -Wno-attributes -Werror" $(MAKE) -j allzstd
237240

238241
ppc64build: clean
239242
CC=powerpc-linux-gnu-gcc CFLAGS="-m64 -Werror" $(MAKE) -j allzstd
240243

244+
ppc64buildCRC: clean
245+
CC=powerpc-linux-gnu-gcc CFLAGS="-m64 -Werror -DZSTD_HASH_USE_CRC32C" $(MAKE) -j allzstd
246+
241247
.PHONY: armfuzz aarch64fuzz ppcfuzz ppc64fuzz
242248
armfuzz: clean
243249
CC=arm-linux-gnueabi-gcc QEMU_SYS=qemu-arm-static MOREFLAGS="-static $(MOREFLAGS)" FUZZER_FLAGS="--no-big-tests $(FUZZER_FLAGS)" $(MAKE) -C $(TESTDIR) fuzztest
@@ -294,6 +300,9 @@ regressiontest:
294300
uasanregressiontest:
295301
$(MAKE) -C $(FUZZDIR) regressiontest CC=clang CXX=clang++ CFLAGS="-O3 -fsanitize=address,undefined -Werror" CXXFLAGS="-O3 -fsanitize=address,undefined -Werror"
296302

303+
uasanregressiontestCRC:
304+
$(MAKE) -C $(FUZZDIR) regressiontest CC=clang CXX=clang++ CFLAGS="-O3 -fsanitize=address,undefined -Werror -DZSTD_HASH_USE_CRC32C" CXXFLAGS="-O3 -fsanitize=address,undefined -Werror"
305+
297306
msanregressiontest:
298307
$(MAKE) -C $(FUZZDIR) regressiontest CC=clang CXX=clang++ CFLAGS="-O3 -fsanitize=memory -Werror" CXXFLAGS="-O3 -fsanitize=memory -Werror"
299308

‎lib/compress/zstd_compress.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2415,7 +2415,7 @@ static void ZSTD_copyCDictTableIntoCCtx(U32* dst, U32 const* src, size_t tableSi
24152415
size_t i;
24162416
for (i = 0; i < tableSize; i++) {
24172417
U32 const taggedIndex = src[i];
2418-
U32 const index = taggedIndex >> ZSTD_SHORT_CACHE_TAG_BITS;
2418+
U32 const index = ZSTD_extractIndex(taggedIndex, ZSTD_SHORT_CACHE_TAG_BITS);
24192419
dst[i] = index;
24202420
}
24212421
} else {

0 commit comments

Comments
 (0)