From b83059958246dfcb5b91af9c187fad8c706869a0 Mon Sep 17 00:00:00 2001 From: mgrice Date: Tue, 27 Aug 2019 14:49:23 -0700 Subject: [PATCH 1/2] Improvements in zstd decode performance Summary: The idea behind wildcopy is that it can be cheaper to copy more bytes (say 8) than it is to copy less (say, 3). This change takes that further by exploiting some properties: 1. it's almost always OK to copy 16 bytes instead of 8, which means fewer copy instructions, and fewer branches 2. A 16 byte chunk size means that ~90% of wildcopy invocations will have a trip count of 1, so branch prediction will be improved. Speedup on Xeon E5-2680v4 is in the range of 3-5%. Measured wildcopy length distributions on silesia.tar: level <=8 <=16 <=24 >24 1 78.05% 11.49% 3.52% 6.94% 3 82.14% 8.99% 2.44% 6.43% 6 85.81% 6.51% 2.92% 4.76% 8 83.02% 7.31% 3.64% 6.03% 10 84.13% 6.67% 3.29% 5.91% 15 77.58% 7.55% 5.21% 9.66% 16 80.07% 7.20% 3.98% 8.75% Test Plan: benchmark silesia, make check --- lib/common/zstd_internal.h | 48 ++++++++++++++++++-------- lib/compress/zstd_compress_internal.h | 2 +- lib/decompress/zstd_decompress_block.c | 12 +++---- 3 files changed, 40 insertions(+), 22 deletions(-) diff --git a/lib/common/zstd_internal.h b/lib/common/zstd_internal.h index 585fd6b19..74769d1d2 100644 --- a/lib/common/zstd_internal.h +++ b/lib/common/zstd_internal.h @@ -191,9 +191,11 @@ static const U32 OF_defaultNormLog = OF_DEFAULTNORMLOG; /*-******************************************* * Shared functions to include for inlining *********************************************/ +FORCE_INLINE_ATTR static void ZSTD_copy8(void* dst, const void* src) { memcpy(dst, src, 8); } #define COPY8(d,s) { ZSTD_copy8(d,s); d+=8; s+=8; } +FORCE_INLINE_ATTR static void ZSTD_copy16(void* dst, const void* src) { memcpy(dst, src, 16); } #define COPY16(d,s) { ZSTD_copy16(d,s); d+=16; s+=16; } @@ -209,7 +211,7 @@ typedef enum { /*! ZSTD_wildcopy() : * custom version of memcpy(), can overwrite up to WILDCOPY_OVERLENGTH bytes (if length==0) */ MEM_STATIC FORCE_INLINE_ATTR DONT_VECTORIZE -void ZSTD_wildcopy(void* dst, const void* src, ptrdiff_t length, ZSTD_overlap_e ovtype) +void ZSTD_wildcopy(void* dst, const void* src, BYTE* oend_g, ptrdiff_t length, ZSTD_overlap_e ovtype) { ptrdiff_t diff = (BYTE*)dst - (const BYTE*)src; const BYTE* ip = (const BYTE*)src; @@ -217,25 +219,33 @@ void ZSTD_wildcopy(void* dst, const void* src, ptrdiff_t length, ZSTD_overlap_e BYTE* const oend = op + length; assert(diff >= 8 || (ovtype == ZSTD_no_overlap && diff < -8)); + if (length < VECLEN || (ovtype == ZSTD_overlap_src_before_dst && diff < VECLEN)) { do COPY8(op, ip) while (op < oend); } else { - if ((length & 8) == 0) - COPY8(op, ip); - do { - COPY16(op, ip); + if (oend < oend_g-16) { + /* common case */ + do { + COPY16(op, ip); + } + while (op < oend); + } + else { + do { + COPY8(op, ip); + } + while (op < oend); } - while (op < oend); } } /*! ZSTD_wildcopy_16min() : - * same semantics as ZSTD_wilcopy() except guaranteed to be able to copy 16 bytes at the start */ + * same semantics as ZSTD_wildcopy() except guaranteed to be able to copy 16 bytes at the start */ MEM_STATIC FORCE_INLINE_ATTR DONT_VECTORIZE -void ZSTD_wildcopy_16min(void* dst, const void* src, ptrdiff_t length, ZSTD_overlap_e ovtype) +void ZSTD_wildcopy_16min(void* dst, const void* src, BYTE* oend_g, ptrdiff_t length, ZSTD_overlap_e ovtype) { ptrdiff_t diff = (BYTE*)dst - (const BYTE*)src; const BYTE* ip = (const BYTE*)src; @@ -246,17 +256,25 @@ void ZSTD_wildcopy_16min(void* dst, const void* src, ptrdiff_t length, ZSTD_over assert(diff >= 8 || (ovtype == ZSTD_no_overlap && diff < -8)); if (ovtype == ZSTD_overlap_src_before_dst && diff < VECLEN) { - do - COPY8(op, ip) + do { + COPY8(op, ip); + } while (op < oend); } else { - if ((length & 8) == 0) - COPY8(op, ip); - do { - COPY16(op, ip); + if (oend < oend_g-16) { + /* common case */ + do { + COPY16(op, ip); + } + while (op < oend); + } + else { + do { + COPY8(op, ip); + } + while (op < oend); } - while (op < oend); } } diff --git a/lib/compress/zstd_compress_internal.h b/lib/compress/zstd_compress_internal.h index 3e590ec37..5fbc68e94 100644 --- a/lib/compress/zstd_compress_internal.h +++ b/lib/compress/zstd_compress_internal.h @@ -359,7 +359,7 @@ MEM_STATIC void ZSTD_storeSeq(seqStore_t* seqStorePtr, size_t litLength, const v /* copy Literals */ assert(seqStorePtr->maxNbLit <= 128 KB); assert(seqStorePtr->lit + litLength <= seqStorePtr->litStart + seqStorePtr->maxNbLit); - ZSTD_wildcopy(seqStorePtr->lit, literals, (ptrdiff_t)litLength, ZSTD_no_overlap); + ZSTD_wildcopy(seqStorePtr->lit, literals, seqStorePtr->lit + litLength + 8, (ptrdiff_t)litLength, ZSTD_no_overlap); seqStorePtr->lit += litLength; /* literal Length */ diff --git a/lib/decompress/zstd_decompress_block.c b/lib/decompress/zstd_decompress_block.c index cbcfc0840..bab96fb7d 100644 --- a/lib/decompress/zstd_decompress_block.c +++ b/lib/decompress/zstd_decompress_block.c @@ -641,7 +641,7 @@ size_t ZSTD_execSequence(BYTE* op, /* copy Literals */ if (sequence.litLength > 8) - ZSTD_wildcopy_16min(op, (*litPtr), sequence.litLength, ZSTD_no_overlap); /* note : since oLitEnd <= oend-WILDCOPY_OVERLENGTH, no risk of overwrite beyond oend */ + ZSTD_wildcopy_16min(op, (*litPtr), oend, sequence.litLength, ZSTD_no_overlap); /* note : since oLitEnd <= oend-WILDCOPY_OVERLENGTH, no risk of overwrite beyond oend */ else ZSTD_copy8(op, *litPtr); op = oLitEnd; @@ -690,13 +690,13 @@ size_t ZSTD_execSequence(BYTE* op, if (oMatchEnd > oend-(16-MINMATCH)) { if (op < oend_w) { - ZSTD_wildcopy(op, match, oend_w - op, ZSTD_overlap_src_before_dst); + ZSTD_wildcopy(op, match, oend, oend_w - op, ZSTD_overlap_src_before_dst); match += oend_w - op; op = oend_w; } while (op < oMatchEnd) *op++ = *match++; } else { - ZSTD_wildcopy(op, match, (ptrdiff_t)sequence.matchLength-8, ZSTD_overlap_src_before_dst); /* works even if matchLength < 8 */ + ZSTD_wildcopy(op, match, oend, (ptrdiff_t)sequence.matchLength-8, ZSTD_overlap_src_before_dst); /* works even if matchLength < 8 */ } return sequenceLength; } @@ -722,7 +722,7 @@ size_t ZSTD_execSequenceLong(BYTE* op, /* copy Literals */ if (sequence.litLength > 8) - ZSTD_wildcopy_16min(op, *litPtr, sequence.litLength, ZSTD_no_overlap); /* note : since oLitEnd <= oend-WILDCOPY_OVERLENGTH, no risk of overwrite beyond oend */ + ZSTD_wildcopy_16min(op, *litPtr, oend, sequence.litLength, ZSTD_no_overlap); /* note : since oLitEnd <= oend-WILDCOPY_OVERLENGTH, no risk of overwrite beyond oend */ else ZSTD_copy8(op, *litPtr); /* note : op <= oLitEnd <= oend_w == oend - 8 */ @@ -772,13 +772,13 @@ size_t ZSTD_execSequenceLong(BYTE* op, if (oMatchEnd > oend-(16-MINMATCH)) { if (op < oend_w) { - ZSTD_wildcopy(op, match, oend_w - op, ZSTD_overlap_src_before_dst); + ZSTD_wildcopy(op, match, oend, oend_w - op, ZSTD_overlap_src_before_dst); match += oend_w - op; op = oend_w; } while (op < oMatchEnd) *op++ = *match++; } else { - ZSTD_wildcopy(op, match, (ptrdiff_t)sequence.matchLength-8, ZSTD_overlap_src_before_dst); /* works even if matchLength < 8 */ + ZSTD_wildcopy(op, match, oend, (ptrdiff_t)sequence.matchLength-8, ZSTD_overlap_src_before_dst); /* works even if matchLength < 8 */ } return sequenceLength; } From 5d897715290883c46aeb1370ba6b54bfd556fa90 Mon Sep 17 00:00:00 2001 From: mgrice Date: Thu, 29 Aug 2019 12:32:15 -0700 Subject: [PATCH 2/2] fix warning: always_inline function might not be inlinable --- lib/common/zstd_internal.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/common/zstd_internal.h b/lib/common/zstd_internal.h index 74769d1d2..fb6246a1e 100644 --- a/lib/common/zstd_internal.h +++ b/lib/common/zstd_internal.h @@ -191,11 +191,9 @@ static const U32 OF_defaultNormLog = OF_DEFAULTNORMLOG; /*-******************************************* * Shared functions to include for inlining *********************************************/ -FORCE_INLINE_ATTR static void ZSTD_copy8(void* dst, const void* src) { memcpy(dst, src, 8); } #define COPY8(d,s) { ZSTD_copy8(d,s); d+=8; s+=8; } -FORCE_INLINE_ATTR static void ZSTD_copy16(void* dst, const void* src) { memcpy(dst, src, 16); } #define COPY16(d,s) { ZSTD_copy16(d,s); d+=16; s+=16; }