Summary:
Freeing an uninitialized pointer is undefined behavior. This caused a segfault
when compiling the benchmark with Clang -O3 and benching decompression.
V2: always create compressInstructions but check if cctxParams is NULL before
setting CCtx params to avoid segfault.
Test Plan:
make and run
Summary:
Added an option -p# where -p0 (default) sets the aggregation method to fastest
speed while -p1 sets the aggregation method to median. Also added a new column
in the csv file to report this option's value.
Test Plan:
``
$ ./largeNbDicts -1 --nbDicts=1 -D ~/benchmarks/html/html_8_16K.32K.dict
~/benchmarks/html/html_8_16K/*
loading 7450 files...
created src buffer of size 83.4 MB
split input into 7450 blocks
loading dictionary /home/zhuhan/benchmarks/html/html_8_16K.32K.dict
compressing at level 1 without dictionary : Ratio=3.03 (28827863 bytes)
compressed using a 32768 bytes dictionary : Ratio=4.28 (20410262 bytes)
generating 1 dictionaries, using 0.1 MB of memory
Compression Speed : 306.0 MB/s
Fastest Speed : 310.6 MB/s
$ ./largeNbDicts -1 --nbDicts=1 -p1 -D ~/benchmarks/html/html_8_16K.32K.dict
~/benchmarks/html/html_8_16K/*
loading 7450 files...
created src buffer of size 83.4 MB
split input into 7450 blocks
loading dictionary /home/zhuhan/benchmarks/html/html_8_16K.32K.dict
compressing at level 1 without dictionary : Ratio=3.03 (28827863 bytes)
compressed using a 32768 bytes dictionary : Ratio=4.28 (20410262 bytes)
generating 1 dictionaries, using 0.1 MB of memory
Compression Speed : 306.9 MB/s
Median Speed : 298.4 MB/s
```
Summary:
Add column headers and data for whether it's a compression or a decompression
run, compression level, nbDicts and dictAttachPref in additional to
compr/decompr speed.
Test Plan:
Example output:
```
./largeNbDicts
Compression/Decompression,Level,nbDicts,dictAttachPref,Speed
Compression,1,1,0,300.9
Compression,1,1,1,296.4
Compression,1,1,2,307.8
Compression,1,10,0,292.3
Compression,1,100,0,293.3
Compression,3,110,0,106.0
Decompression,-1,110,-1,155.6
Decompression,-1,110,-1,709.4
Decompression,-1,120,-1,709.1
Decompression,-1,120,-1,734.6
```
Benchmarking decompression results in a segfault in `createCompressInstructions`
because `cctxParams` is NULL. Skip running that function if we are not benching
compression.
- As referenced by Nick Terrelln ~ the ZSTD maintainer in the linux kernel, making zstd_reset_cstream() functionally identical to ZSTD_resetCStream() would be the perfect way to fix the warning without touching any core functions or breaking other parts of the code.
Suggested-by: Nick Terrell <terrelln@fb.com>
Signed-off-by: Cyber Knight <cyberknight755@gmail.com>
- The previous patch throws the following warning:
../linux/lib/zstd/zstd_compress_module.c: In function ‘zstd_reset_cstream’:
../linux/lib/zstd/zstd_compress_module.c:136:34: error: enum conversion when passing argument 2 of ‘ZSTD_CCtx_reset’ is invalid in C++ [-Werror=c++-compat]
136 | return ZSTD_CCtx_reset(cstream, pledged_src_size);
| ^~~~~~~~~~~~~~~~
In file included from ../linux/include/linux/zstd.h:26,
from ../linux/lib/zstd/zstd_compress_module.c:15:
../linux/include/linux/zstd_lib.h:501:20: note: expected ‘ZSTD_ResetDirective’ {aka ‘enum <anonymous>’} but argument is of type ‘long long unsigned int’
501 | ZSTDLIB_API size_t ZSTD_CCtx_reset(ZSTD_CCtx* cctx, ZSTD_ResetDirective reset);
| ^~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
Since we have a choice to either use ZSTD_CCtx_reset or ZSTD_CCtx_setPledgedSrcSize instead of ZSTD_resetCStream, let's switch to ZSTD_CCtx_setPledgedSrcSize to not have any unnecessary warns alongside the kernel build and CI test build.
Signed-off-by: Cyber Knight <cyberknight755@gmail.com>
- This fixes the below warning:
../lib/zstd/zstd_compress_module.c: In function 'zstd_reset_cstream':
../lib/zstd/zstd_compress_module.c:136:9: warning: 'ZSTD_resetCStream' is deprecated [-Wdeprecated-declarations]
136 | return ZSTD_resetCStream(cstream, pledged_src_size);
| ^~~~~~
In file included from ../include/linux/zstd.h:26,
from ../lib/zstd/zstd_compress_module.c:15:
../include/linux/zstd_lib.h:2277:8: note: declared here
2277 | size_t ZSTD_resetCStream(ZSTD_CStream* zcs, unsigned long long pledgedSrcSize);
| ^~~~~~~~~~~~~~~~~
ZSTD_resetCstream is deprecated and zstd_CCtx_reset is suggested to use hence let's switch to it.
Signed-off-by: Cyber Knight <cyberknight755@gmail.com>
* Refactored fileio.c:
- Extracted asyncio code to fileio_asyncio.c/.h
- Moved type definitions to fileio_types.h
- Moved common macro definitions needed by both fileio.c and fileio_asyncio.c to fileio_common.h
* Bugfix - rename fileio_asycio to fileio_asyncio
* Added copyrights & license to new files
* CR fixes
Move portability macros to `lib/common/portability_macros.h`. This file
only contains platform/feature detection (e.g. 0/1 macros). This file is
shared between C and ASM code, so it cannot include any C code.
Rename `HUF_` ASM macros to be `ZSTD_` prefixed, and move to the new
header.
Restrict `ZSTD_ASM_SUPPORTED` to `__GNUC__`, because we need the GAS
assembler.
Finally, only include the ASM code if we are actually going to use it.
This disables it on all Windows platforms, which should resolve the
problem brought up in Issue #2789.
gcc-5 didn't like the l-value overload for defaulted operator=. There is
no reason it needs to be l-value overloaded, so just remove it.
I'm not sure why the build broke for @mckaygerhard in Issue #2811, since
this code hasn't changed since it was added. But, there is no harm in
fixing it.
Fixes issue #2811.
It is no longer necessary to get good performance, there is only a small
speed difference between -O2 and -O3, so just stick to the default of
-O2. I've measured neutral compression speed and a ~3% decompression
speed loss in userspace with clang & gcc. I've also measured neutral
compression speed and a ~1% decompression speed loss in the kernel
benchmarks.
This also fixes the stack space usage on parisc. The compiler was buggy
for -O3 and used ~3KB of stack space for several functions. With -O2 the
problem is completely resolved, and stack space is back to a few hundred
bytes.
Additionally, we get a large code size win on gcc:
| Compiler | Before (Bytes) | After (Bytes) | Delta (Bytes) |
|----------|----------------|---------------|---------------|
| gcc-11 | 952754 | 738954 | -213800 |
| clang-12 | 976290 | 938826 | -37464 |
The optimal parser is unlikely to be used in the linux kernel in
practice. There is no reason these functions should be force inlined,
since we aren't gaining anything, and are losing build size.
| Compiler | Before (Bytes) | After (Bytes) | Delta (Bytes) |
|----------|----------------|---------------|---------------|
| gcc-11 | 1142090 | 952754 | -189336 |
| clang-12 | 1228402 | 976290 | -252112 |
This is a temporary solution pending the resolution of PR #2862 in the
`dev` branch.
Test the kernel build with the standard warnings enabled so that we
don't miss issues like fixed in PR #2802.
Stacked on top of PR #2802 so CI passes, so it includes the fix. But, I
won't merge until it is merged, so @solbjorn gets the credit for the
fix.
Switch to a macro `ZSTD_FALLTHROUGH;` instead of a comment. On supported
compilers this uses an attribute, otherwise it becomes a comment.
This is necessary to be compatible with clang's `-Wfall-through`, and
gcc's `-Wfall-through=2` which don't support comments. Without this the
linux build emits a bunch of warnings.
Also add a test to CI to ensure that we don't regress.
Commit a5f2c4552803 ("Huffman ASM") added a new ASM source file,
but it wasn't added to the kernel Makefile despite that it received
support for Huffman ASM according to the internal definitions. This
leads to undefined references, as huf_decompress.o now calls those
ASM functions.
Add it to the list of sources when building inside the kernel tree.
Kbuild can handle .S files just fine, so none additional rules
needed.
Fixes: a5f2c4552803 ("Huffman ASM")
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
Linux 5.15 introduces a new Kconfig option, CONFIG_WERROR, which
forces -Werror for the entire kernel.
Current in-kernel ZSTD implementation uses functions deprecated
in 1.5.0, and thus fails on -Wdeprecated-declarations.
Turn this particular error into warning to be able to build the
kernel with CONFIG_WERROR. I'm not disabling them completely to
make sure they'll be visible and [hopefully] fixed sooner or later.
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
Instead of calling `ZSTD_compress_advanced()` and
`ZSTD_initCStream_advanced()`, which each take a `ZSTD_parameters` by
value, use the new advanced API.
Stack usage went from 2024 -> 1944.
* Extract out common portion of `lib/Makefile` into `lib/libzstd.mk`.
Most relevantly, the way we find library files.
* Use `lib/libzstd.mk` in the other Makefiles instead of repeating the
same code.
* Add a test `tests/test-variants.sh` that checks that the builds of
`make -C programs allVariants` are correct, and run it in Actions.
* Adds support for ASM files in the CMake build.
The Meson build is not updated because it lists every file in zstd,
and supports ASM off the bat, so the Huffman ASM commit will just add
the ASM file to the list.
The Visual Studios build is not updated because I'm not adding ASM
support to Visual Studios yet.
* seekable_format: fix from-file reading (not in-memory)
It tries to check the buffer boundary, but there is no buffer for
from-file reading.
* seekable_decompression: break when ZSTD_seekable_decompress() returns zero
* seekable_decompression_mem: break when ZSTD_seekable_decompress() returns zero
* seekable_format: cap the offset+len up to the last dOffset
This will allow to read the whole file w/o gotting corruption error if
the offset is more then the data left in file, i.e.:
$ ./seekable_compression seekable_compression.c 8192 | head
$ zstd -cdq seekable_compression.c.zst | wc -c
4737
Before this patch:
$ ./seekable_decompression seekable_compression.c.zst 0 10000000 | wc -c
ZSTD_seekable_decompress() error : Corrupted block detected
0
After:
$ ./seekable_decompression seekable_compression.c.zst 0 10000000 | wc -c
4737
`zstd_errors.h` and `zdict.h` are public headers, so they deserve to be
in the root `lib/` directory with `zstd.h`, not mixed in with our private
headers.
Replace kernel-style comments with regular comments.
E.g.
```
/** Before */
/* After */
/**
* Before
*/
/*
* After
*/
/***********************************
* Before
***********************************/
/* *********************************
* After
***********************************/
```
* Switch to yearless copyright per FB policy
* Fix up SPDX-License-Identifier lines in `contrib/linux-kernel` sources
* Add zstd copyright/license header to the `contrib/linux-kernel` sources
* Update the `tests/test-license.py` to check for yearless copyright
* Improvements to `tests/test-license.py`
* Check `contrib/linux-kernel` in `tests/test-license.py`
