Add dedicated sanity test program

t-mat commented

2023-03-09 16:49:35 -06:00

(Migrated from github.com)

To detect a bug like #816 , we need more complete and dedicated sanity test program.

👍 2

easyaspi314 commented

2023-03-09 17:19:51 -06:00

(Migrated from github.com)

In addition, this could be an opportunity to make the quick bench more lightweight by decreasing the initial number of iterations to something low like 10 MB/s which can greatly improve performance if the target cannot reach high speeds, and automatically adjusts on faster targets. This is easy to notice when using qemu, -O0, or sanitizers.

As an extreme example, using my pretty crappy 1.5 GHz Cortex-A53 tablet with s390x in QEMU with -O0 -mzvector, ./xxhsum -b99i0 took 38 minutes, as the combination of the unoptimized code and the emulation layer brought it to 3 MB/s, meaning each iteration was about 100 seconds.

For the more accurate and thorough benchmarks, we have benchHash, and the same should be for the self tests.

In addition, this could be an opportunity to make the quick bench more lightweight by decreasing the initial number of iterations to something low like 10 MB/s which can greatly improve performance if the target cannot reach high speeds, and automatically adjusts on faster targets. This is easy to notice when using `qemu`, `-O0`, or sanitizers. As an extreme example, using my pretty crappy 1.5 GHz Cortex-A53 tablet with `s390x` in QEMU with `-O0 -mzvector`, `./xxhsum -b99i0` took 38 minutes, as the combination of the unoptimized code and the emulation layer brought it to 3 MB/s, meaning each iteration was about 100 seconds. ![Screenshot_20230308-125702.png](https://user-images.githubusercontent.com/6258309/224182413-930968fd-5d6a-4f5c-9bf3-9611c319f026.png) For the more accurate and thorough benchmarks, we have benchHash, and the same should be for the self tests.

👍 2

t-mat commented

2023-03-10 08:14:19 -06:00

(Migrated from github.com)

I updated test vector generator and sanity test program.

We can test them with the following commands:

git clone https://github.com/Cyan4973/xxHash.git xxHash-issue-821-2023-0310
cd xxHash-issue-821-2023-0310
cd tests

curl -JOL https://gist.githubusercontent.com/t-mat/168b85d35af11c17ad5953317ff3f2d9/raw/sanity_test_vectors_generator.c
curl -JOL https://gist.githubusercontent.com/t-mat/6db94988ff5dcb4a979c1291144f7154/raw/sanity_test.c

# Generate sanity_test_vectors.h (HUGE! 😮 )
cc sanity_test_vectors_generator.c && ./a.out

# Actual sanity test
cc sanity_test.c && ./a.out

If it seems okay as a version 0, TODOs are

Write test vector generator and sanity test function for secret generator.
Make sure it tests and provides good test vectors to prevent similar error.
Add target for make check.
Tweak compatibility with xsum_sanity_check.c (should we care about it?)
Tweak compatibility with QEMU platforms. (Definitely we need it)

I updated test vector generator and sanity test program. We can test them with the following commands: ```sh git clone https://github.com/Cyan4973/xxHash.git xxHash-issue-821-2023-0310 cd xxHash-issue-821-2023-0310 cd tests curl -JOL https://gist.githubusercontent.com/t-mat/168b85d35af11c17ad5953317ff3f2d9/raw/sanity_test_vectors_generator.c curl -JOL https://gist.githubusercontent.com/t-mat/6db94988ff5dcb4a979c1291144f7154/raw/sanity_test.c # Generate sanity_test_vectors.h (HUGE! 😮 ) cc sanity_test_vectors_generator.c && ./a.out # Actual sanity test cc sanity_test.c && ./a.out ``` If it seems okay as a version 0, TODOs are - [x] Write test vector generator and sanity test function for secret generator. - [x] Make sure it tests and provides good test vectors to prevent similar error. - [x] Add target for `make check`. - [ ] Tweak compatibility with `xsum_sanity_check.c` (should we care about it?) - [x] Tweak compatibility with QEMU platforms. (Definitely we need it)

👍 1

Cyan4973 commented

2023-03-10 16:46:19 -06:00

(Migrated from github.com)

Yes, this is pretty good @t-mat !

Cyan4973 commented

2023-03-10 16:47:49 -06:00

(Migrated from github.com)

Tweak compatibility with xsum_sanity_check.c (should we care about it?)

I'm not sure if that's necessary.
What kind of compatibility seems desirable ?

> Tweak compatibility with xsum_sanity_check.c (should we care about it?) I'm not sure if that's necessary. What kind of compatibility seems desirable ?

t-mat commented

2023-03-10 19:30:56 -06:00

(Migrated from github.com)

Tweak compatibility with xsum_sanity_check.c (should we care about it?)

What kind of compatibility seems desirable ?

Sorry, I didn't express what I think.

So far, test functions of sanity_test.c are copied (but slightly modified) from xsum_sanity_check.c.
But I think we can re-use XSUM_test*() functions to avoid to copy relatively complex functions.

To reuse them, I think the following changes are needed.

(1) Avoid pseudo global-mutable variables. These variables should be provided from test driver (XSUM_sanityCheck()).

XSUM_checkResult*().nbTests
XSUM_rand().seed

(2) Tweak error message. It should avoid "global" test number. It may look like this

Error: xsum_sanity_check.c, line 390.
  Vector = XSUM_XXH3_testdata[8]
  Got ..., expected ...

> > Tweak compatibility with xsum_sanity_check.c (should we care about it?) > What kind of compatibility seems desirable ? Sorry, I didn't express what I think. So far, test functions of `sanity_test.c` are copied (but slightly modified) from `xsum_sanity_check.c`. But I think we can re-use `XSUM_test*()` functions to avoid to copy relatively complex functions. To reuse them, I think the following changes are needed. (1) Avoid pseudo global-mutable variables. These variables should be provided from test driver (`XSUM_sanityCheck()`). - `XSUM_checkResult*().nbTests` - `XSUM_rand().seed` (2) Tweak error message. It should avoid "global" test number. It may look like this ``` Error: xsum_sanity_check.c, line 390. Vector = XSUM_XXH3_testdata[8] Got ..., expected ... ```

Cyan4973 commented

2023-03-10 19:31:58 -06:00

(Migrated from github.com)

So far, test functions of sanity_test.c are copied (but slightly modified) from xsum_sanity_check.c.
But I think we can re-use XSUM_test*() functions to avoid to copy relatively complex functions.

Yes, this is definitely a good idea.

> So far, test functions of `sanity_test.c` are copied (but slightly modified) from `xsum_sanity_check.c`. > But I think we can re-use `XSUM_test*()` functions to avoid to copy relatively complex functions. Yes, this is definitely a good idea.

👍 1

t-mat commented

2023-07-01 09:33:48 -05:00

(Migrated from github.com)

I think we can close it for now. We may revisit this issue (duplicated code). But I belive we'll do it after new release and speratation/simplified code base.

👍 1

flawrist commented

2026-01-31 14:31:41 -06:00

(Migrated from github.com)

Setup: OS Windows 7 x64, xxhsum v0.8.3.

I'm confused with this sanity check. Please, give me a hand.

From https://github.com/Cyan4973/xxHash/blob/dev/tests/sanity_test_vectors.h

    {   14, 0x00000000U, 0x1208E7E2U }, /* XSUM_XXH32_testdata[28] */

Ok, I create zero-filled 14 bytes file, then calculate its xxh32 hash and…

$ busybox.exe dd if=/dev/zero of=zero14.txt bs=14 count=1
1+0 records in
1+0 records out

$ busybox.exe du -b zero14.txt
14      zero14.txt

$ xxhsum.exe -H0 zero14.txt
7974215b test.txt

Mismatch! What am I doing wrong?

cc: @Cyan4973

Setup: OS Windows 7 x64, xxhsum v0.8.3. I'm confused with this sanity check. Please, give me a hand. From https://github.com/Cyan4973/xxHash/blob/dev/tests/sanity_test_vectors.h <pre> { 14, 0x00000000U, 0x<mark>1208E7E2</mark>U }, /* XSUM_XXH32_testdata[28] */ </pre> Ok, I create zero-filled 14 bytes file, then calculate its xxh32 hash and… <pre> $ busybox.exe dd if=/dev/zero of=zero14.txt bs=14 count=1 1+0 records in 1+0 records out $ busybox.exe du -b zero14.txt 14 zero14.txt $ xxhsum.exe -H0 zero14.txt <mark>7974215b</mark> test.txt </pre> Mismatch! What am I doing wrong? cc: @Cyan4973

Rows
Columns

Add dedicated sanity test program #821