xxhash can be inlined even if it was previously included

Inlining hash functions is generally beneficial for performance.
It becomes extremely beneficial whenever input size is a compile-time constant.

To inline xxhash functions, one just needs to include it this way :

One potential issue is that "xxhash.h" may have already been included previously,
typically as part of another included `*.h` .
In which case, the second `#include` statement will have no effect.

This patch fixes this situation :
now, when XXH_INLINE_ALL is defined, all identifiers are renamed,
in order to avoid name collision and confusion with already included indentifiers.

The renaming process uses XXH_NAMESPACE.
XXH_NAMESPACE must be available (i.e. not already used) for renaming to work.

A test has been added in `tests/` to ensure this scenario works correctly.
This commit is contained in:
Yann Collet 2020-02-12 11:43:33 -08:00
parent 8ae09480cf
commit 19cd4ffed3
6 changed files with 169 additions and 19 deletions

View File

@ -282,9 +282,13 @@ preview-man: man
test: DEBUGFLAGS += -DDEBUGLEVEL=1
test: all namespaceTest check test-xxhsum-c c90test test-tools
.PHONY: test-inline
test-inline:
$(MAKE) -C tests test
.PHONY: test-all
test-all: CFLAGS += -Werror
test-all: test test32 clangtest cxxtest usan listL120 trailingWhitespace staticAnalyze
test-all: test test32 clangtest cxxtest usan test-inline listL120 trailingWhitespace staticAnalyze
.PHONY: test-tools
test-tools:

45
tests/Makefile Normal file
View File

@ -0,0 +1,45 @@
CFLAGS += -Wall -Wextra -g
NM = nm
GREP = grep
.PHONY: default
default: all
.PHONY: all
all: test
.PHONY: test
test: test_multiinclude
.PHONY: test_multiinclude
test_multiinclude:
@$(MAKE) clean
# compile without xxhash.o, ensure symbols exist within target
# note : built using only default rules
$(MAKE) multiInclude
@$(MAKE) clean
# compile with xxhash.o, to detect duplicated symbols
$(MAKE) multiInclude_withxxhash
@$(MAKE) clean
# Note : XXH_INLINE_ALL with XXH_NAMESPACE is currently disabled
# compile with XXH_NAMESPACE
# CPPFLAGS=-DXXH_NAMESPACE=TESTN_ $(MAKE) multiInclude_withxxhash
# no symbol prefixed TESTN_ should exist
# ! $(NM) multiInclude_withxxhash | $(GREP) TESTN_
#$(MAKE) clean
# compile with XXH_NAMESPACE and without xxhash.o
# CPPFLAGS=-DXXH_NAMESPACE=TESTN_ $(MAKE) multiInclude
# no symbol prefixed TESTN_ should exist
# ! $(NM) multiInclude | $(GREP) TESTN_
#@$(MAKE) clean
xxhash.o: ../xxhash.c ../xxhash.h
$(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) -c -o $@ $<
multiInclude_withxxhash: multiInclude.o xxhash.o
$(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) -o $@ $^
clean:
@$(RM) *.o
@$(RM) multiInclude multiInclude_withxxhash

58
tests/multiInclude.c Normal file
View File

@ -0,0 +1,58 @@
/*
* test_multiinclude
* test program, just to validate including multiple times in any order
*
* Copyright (C) Yann Collet 2013-present
*
* GPL v2 License
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License along
* with this program; if not, write to the Free Software Foundation, Inc.,
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*
* You can contact the author at :
* - xxHash homepage : http://www.xxhash.com
* - xxHash source repository : https://github.com/Cyan4973/xxHash
*/
#include <stdio.h> // printf
/* normal include, gives access to public symbols */
#include "../xxhash.h"
/* advanced include, gives access to experimental symbols */
/* note : without specific effort, experimental symbols might be unavailable */
#define XXH_STATIC_LINKING_ONLY
#include "../xxhash.h"
/* inlining : re-define all symbols, keep them private to the unit.
* note : without specific efforts, symbol names will collide
* note 2 : no linking to xxhash.o, so that link stage will fail if inline is ignored */
#define XXH_INLINE_ALL
#include "../xxhash.h"
int main(void)
{
XXH3_state_t state; /* part of experimental */
XXH3_64bits_reset(&state);
const char input[] = "Hello World !";
XXH3_64bits_update(&state, input, sizeof(input));
XXH64_hash_t const h = XXH3_64bits_digest(&state);
printf("hash '%s' : %0llx \n", input, (unsigned long long)h);
return 0;
}

2
xxh3.h
View File

@ -1654,7 +1654,7 @@ XXH_PUBLIC_API XXH128_hash_t XXH3_128bits_digest (const XXH3_state_t* state)
/* 128-bit utility functions */
#include <string.h> /* memcmp */
#include <string.h> /* memcmp, memcpy */
/* return : 1 is equal, 0 if different */
XXH_PUBLIC_API int XXH128_isEqual(XXH128_hash_t h1, XXH128_hash_t h2)

View File

@ -33,7 +33,7 @@
*/
/* xxhash.c only instantiates functions defined in xxhash.h
/* xxhash.c instantiates functions defined in xxhash.h
*/
#define XXH_STATIC_LINKING_ONLY /* access advanced declarations */

View File

@ -73,29 +73,29 @@ XXH32 6.8 GB/s 6.0 GB/s
extern "C" {
#endif
#ifndef XXHASH_H_5627135585666179
#define XXHASH_H_5627135585666179 1
/* ****************************
* API modifier
******************************/
/** XXH_INLINE_ALL (and XXH_PRIVATE_API)
* This build macro includes xxhash functions in `static` mode
* in order to inline them, and remove their symbol from the public list.
* Inlining offers great performance improvement on small keys,
* Implement requested xxhash functions directly in the unit.
* Inlining offers great performance improvement on small inputs,
* and dramatic ones when length is expressed as a compile-time constant.
* See https://fastcompression.blogspot.com/2018/03/xxhash-for-small-keys-impressive-power.html .
* It also removes all symbols from the public list.
* Methodology :
* #define XXH_INLINE_ALL
* #include "xxhash.h"
* `xxhash.c` is automatically included.
* It's not useful to compile and link it as a separate object.
* Do not compile and link xxhash.o as a separate object (not useful)
*/
#if defined(XXH_INLINE_ALL) || defined(XXH_PRIVATE_API)
# ifndef XXH_STATIC_LINKING_ONLY
#if (defined(XXH_INLINE_ALL) || defined(XXH_PRIVATE_API)) \
&& !defined(XXH_INLINE_ALL_31684351384)
/* this section should be traversed only once */
# define XXH_INLINE_ALL_31684351384
/* give access to advanced API, required to compile implementations */
# undef XXH_STATIC_LINKING_ONLY /* avoid macro redef */
# define XXH_STATIC_LINKING_ONLY
# endif
/* make functions private */
# undef XXH_PUBLIC_API
# if defined(__GNUC__)
# define XXH_PUBLIC_API static __inline __attribute__((unused))
# elif defined (__cplusplus) || (defined (__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L) /* C99 */)
@ -103,10 +103,48 @@ extern "C" {
# elif defined(_MSC_VER)
# define XXH_PUBLIC_API static __inline
# else
/* this version may generate warnings for unused static functions */
/* note : this version may generate warnings for unused static functions */
# define XXH_PUBLIC_API static
# endif
#else
/* prefix all names, to avoid symbol duplicates with potential library */
# ifdef XXH_NAMESPACE
# error "XXH_INLINE_ALL with XXH_NAMESPACE is not supported"
# /* Note : Alternative is to #undef all symbols (it's a pretty large list).
* If doing nothing : it compiles, but functions are actually Not inlined.
* */
# endif
# define XXH_NAMESPACE XXH_INLINE_
/* some identifiers are not symbols,
* they must nonetheless be renamed to avoid double declaration
* Alternative : do not redeclare them,
* which requires some #ifdef, and is more dispersed in the file
* while renaming can be achieved in a single place */
# define XXH_IPREF(Id) XXH_INLINE_ ## Id
# define XXH_OK XXH_IPREF(XXH_OK)
# define XXH_ERROR XXH_IPREF(XXH_ERROR)
# define XXH_errorcode XXH_IPREF(XXH_errorcode)
# define XXH32_canonical_t XXH_IPREF(XXH32_canonical_t)
# define XXH64_canonical_t XXH_IPREF(XXH64_canonical_t)
# define XXH128_canonical_t XXH_IPREF(XXH128_canonical_t)
# define XXH32_state_s XXH_IPREF(XXH32_state_s)
# define XXH32_state_t XXH_IPREF(XXH32_state_t)
# define XXH64_state_s XXH_IPREF(XXH64_state_s)
# define XXH64_state_t XXH_IPREF(XXH64_state_t)
# define XXH3_state_s XXH_IPREF(XXH3_state_s)
# define XXH3_state_t XXH_IPREF(XXH3_state_t)
# define XXH128_hash_t XXH_IPREF(XXH128_hash_t)
/* Ensure header is parsed again, even if it was previously included */
# undef XXHASH_H_5627135585666179
# undef XXHASH_H_STATIC_13879238742
#endif /* XXH_INLINE_ALL || XXH_PRIVATE_API */
#ifndef XXHASH_H_5627135585666179
#define XXHASH_H_5627135585666179 1
/* specific declaration modes for Windows */
#if !defined(XXH_INLINE_ALL) && !defined(XXH_PRIVATE_API)
# if defined(WIN32) && defined(_MSC_VER) && (defined(XXH_IMPORT) || defined(XXH_EXPORT))
# ifdef XXH_EXPORT
# define XXH_PUBLIC_API __declspec(dllexport)
@ -116,7 +154,7 @@ extern "C" {
# else
# define XXH_PUBLIC_API /* do nothing */
# endif
#endif /* XXH_INLINE_ALL || XXH_PRIVATE_API */
#endif
/*! XXH_NAMESPACE, aka Namespace Emulation :
*
@ -577,9 +615,14 @@ XXH_PUBLIC_API XXH128_hash_t XXH128_hashFromCanonical(const XXH128_canonical_t*
#endif /* defined(XXH_STATIC_LINKING_ONLY) && !defined(XXHASH_H_STATIC_13879238742) */
/* ======================================================================== */
/* ======================================================================== */
/* ======================================================================== */
/*-**********************************************************************
* xxHash implementation
* -**********************************************************************
* Functions implementation used to be hosted within xxhash.c .
* However, code inlining requires to place implementation in the header file.
* As a consequence, xxhash.c used to be included within xxhash.h .