XXH3_64bits speed testing

[aleksazr]

Hi, I'm just starting with xxHash, mainly I'm interested in XXH3_64bits, SSE2.

I've started by testing the speed in streaming mode, byte by byte:

#define XXH_INLINE_ALL
#define XXH_STATIC_LINKING_ONLY
#define XXH_FORCE_MEMORY_ACCESS 2
#define XXH_CPU_LITTLE_ENDIAN 1
#define XXH_NO_STDLIB
#define XXH_DEBUGLEVEL 0
#define XXH_VECTOR XXH_SSE2

XXH3_state_t state;
XXH3_64bits_reset(&state);

for (int f = 0; f < 100000000; f++) {
static const xxh_u8 data = 0xAA;
XXH3_64bits_update(&state, &data, 1);
}

XXH64_hash_t rez = XXH3_64bits_digest(&state);

Q1: is this how I'm supposed to use it?

I've tested it with XXH3_STREAM_USE_STACK=1 and =0, and the result I get is 940/765 ms.
So, the =0 is a bit faster, in this case.

Then I started changing XXH3_update(), and made 3 versions:

#include "xxhash 0.h" // (940/765 ms) original xxhash.h file
#include "xxhash 1.h" // (940/765 ms) no return value
#include "xxhash 2.h" // (815/695 ms) move down
#include "xxhash 3.h" // (424/450 ms) local memcpy

xxhash 0.h
the original file, where I get 940ms for XXH3_STREAM_USE_STACK=1, and 765ms for XXH3_STREAM_USE_STACK=0.

xxhash 1.h
I've noticed that XXH3_update() always returns XXH_OK, so I've changed it into a void return.
That did not speed it up at all, but why returning it?

xxhash 2.h
XXH_memcpy(acc, state->acc, sizeof(acc));
this line should be moved down, after "small input : just fill in tmp buffer" part.
MSVC compiler copies the state->acc into stack acc, even if it won't be used - wasting time.
The same goes for bEnd and secret.
That resulted in 815/695 ms.

xxhash 3.h
"small input : just fill in tmp buffer" uses memcpy which has a lot of overhead, at least my version does.
So I've changed it to copy less than 8 bytes locally, and that resulted in 424/450 ms (double the speed of xxhash 0.h)
(the only case in these tests where XXH3_STREAM_USE_STACK=0 is slower)
8 bytes could be #define'd as the user chooses.

I've attached all files, so they can be easily compared to each other.
xxhash.zip

Would like to hear some thoughts about this.

[Cyan4973]

Q1: is this how I'm supposed to use it?

Yes

xxhash 1.h
I've noticed that XXH3_update() always returns XXH_OK, so I've changed it into a void return.
That did not speed it up at all, but why returning it?

This is for API consistency.
This way, XXH3_update() looks like all other XXHnn_update().

When this API was initially settled, it felt that an error signal could be useful for complex scenarios.
Many years later, the outcome is not as anticipated, and we could be dispensed from the return code after all.
But too late.

xxhash 2.h

nice optimization !

As a minor detail, as is, the code does not respect C90 declaration order,
but this is easy to fix.

xxhash 3.h

This one looks less obvious to me.
It adds complexity for the benefit of an unlikely corner case scenario.

Moreover, while it works fine here because it copies precisely 1 byte every time,
if, instead, the amount is random between 1 and 7,
I'm less certain that it would remain a good choice.
And yes, I'm aware this alternative scenario is also unlikely.
In both cases, it's questionable if it's worth adding complexity to optimize for them.