xxh_x86dispatch.h not installed when DISPATCH=1 #1058

Closed
opened 2025-08-12 21:40:06 -05:00 by xiota · 5 comments
xiota commented 2025-08-12 21:40:06 -05:00 (Migrated from github.com)

xxh_x86dispatch.h is not installed by make DISPATCH=1 install.

It is installed by make DISPATCH=1 LIBXXH_DISPATCH=1 install.

I suspect a copy/paste error?

github.com/Cyan4973/xxHash@7aee8d0a34/Makefile (L633-L635)

Should be:

ifeq ($(DISPATCH),1)
	$(INSTALL_DATA) xxh_x86dispatch.h $(DESTDIR)$(INCLUDEDIR)
endif
`xxh_x86dispatch.h` is not installed by `make DISPATCH=1 install`. It is installed by `make DISPATCH=1 LIBXXH_DISPATCH=1 install`. I suspect a copy/paste error? https://github.com/Cyan4973/xxHash/blob/7aee8d0a341bb574f7c139c769e1db115b42cc3c/Makefile#L633-L635 Should be: ```make ifeq ($(DISPATCH),1) $(INSTALL_DATA) xxh_x86dispatch.h $(DESTDIR)$(INCLUDEDIR) endif ```
xiota commented 2025-08-15 20:42:19 -05:00 (Migrated from github.com)

Feel free to close if this is intended behavior. From Readme:

LIBXXH_DISPATCH=1 : same idea, implemented a runtime vector extension detector, but within libxxhash. This parameter is disabled by default. When enabled (only valid for x86/x64 systems), new symbols published in xxh_x86dispatch.h become accessible. At the time of this writing, it's required to include xxh_x86dispatch.h in order to access the symbols with runtime vector extension detection.

Feel free to close if this is intended behavior. From Readme: > LIBXXH_DISPATCH=1 : same idea, implemented a runtime vector extension detector, but within libxxhash. This parameter is disabled by default. When enabled (only valid for x86/x64 systems), new symbols published in xxh_x86dispatch.h become accessible. At the time of this writing, it's required to include xxh_x86dispatch.h in order to access the symbols with runtime vector extension detection.
eworm-de commented 2025-08-16 02:54:42 -05:00 (Migrated from github.com)

You could create a pull request for this to speed up things...

You could create a pull request for this to speed up things...
xiota commented 2025-08-16 10:19:46 -05:00 (Migrated from github.com)

The naive expectation would be for everything to work under the DISPATCH flag, but keeping LIBXXH_DISPATCH separate appears to have been intentional. (I'd missed the note in the readme when I opened this issue.)

Upon reflection, there shouldn't be any need for third-party projects to use the dispatch header, so maybe ability to install it should be removed from the Makefile entirely? (Or comment added to note it's for internal use/testing only.)

The project I encountered using the dispatch header was likely doing so incorrectly. It does not use any of the exported dispatch symbols.

The naive expectation would be for everything to work under the DISPATCH flag, but keeping LIBXXH_DISPATCH separate appears to have been intentional. (I'd missed the note in the readme when I opened this issue.) Upon reflection, there shouldn't be any need for third-party projects to use the dispatch header, so maybe ability to install it should be removed from the Makefile entirely? (Or comment added to note it's for internal use/testing only.) The project I encountered using the dispatch header was likely doing so incorrectly. It does not use any of the exported dispatch symbols.
eworm-de commented 2025-08-16 13:36:31 -05:00 (Migrated from github.com)

Then let's wait of upstream to respond, and I will keep the downstream package as is.

Then let's wait of upstream to respond, and I will keep the downstream package as is.
Cyan4973 commented 2026-09-16 21:43:19 -05:00 (Migrated from github.com)

Yeah, so, indeed, DISPATCH=1 is specifically for the CLI xxhsum.
The dispatch is useful for a case like xxhsum, because it's a binary compiled once and distributed across a large amount of heterogeneous machines. So it allows runtime adaptation. It's also expected to hash "large" content (at least KB range, frequently more). So that's where dispatch offers most benefits.

However, for the library libxxhash, I expect the dispatch to be less useful, if at all.
The expectation is that it's employed in drastically different scenarios, such as hash tables or bloom filter, and the input to hash might be small, if not tiny, or even fixed size. In which case, inlining optimization become important for performance. And dispatch makes things more complex, it can get in the way. At least it makes things more complex for the compiler.
Hence that's why the policy is different.

That being said, just in case someone would think that it's nonetheless useful for their specific usecase, there is an opt-in, with LIBXXH_DISPATCH=1.

Yeah, so, indeed, `DISPATCH=1` is specifically for the CLI `xxhsum`. The dispatch is useful for a case like `xxhsum`, because it's a binary compiled once and distributed across a large amount of heterogeneous machines. So it allows runtime adaptation. It's also expected to hash "large" content (at least KB range, frequently more). So that's where dispatch offers most benefits. However, for the library `libxxhash`, I expect the dispatch to be less useful, if at all. The expectation is that it's employed in drastically different scenarios, such as hash tables or bloom filter, and the input to hash might be small, if not tiny, or even fixed size. In which case, inlining optimization become important for performance. And dispatch makes things more complex, it can get in the way. At least it makes things more complex for the compiler. Hence that's why the policy is different. That being said, just in case someone would think that it's nonetheless useful for their specific usecase, there is an opt-in, with `LIBXXH_DISPATCH=1`.
Sign in to join this conversation.
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-Cyan4973/xxHash#1058
No description provided.