Re: [PATCH] zlib: Fix big performance regression

On Thu, Jun 27, 2024 at 10:25:21AM +0200, Christophe Leroy wrote:

Commit 340fdf1303dc ("zlib: Port fix for CVE-2016-9841 to U-Boot") brings a big performance regression in inflate_fast(), which leads to watchdog timer reset on powerpc 8xx.

It looks like that commit does more than what it describe, it especially removed an important optimisation that was doing copies using halfwords instead of bytes. That unexpected change multiplied by almost 4 the time spent in inflate_fast() and increased by 40% the overall time needed to uncompress linux kernel image.

So partially revert that commit but keep post incrementation as it is the initial purpose of said commit.

Fixes: 340fdf1303dc ("zlib: Port fix for CVE-2016-9841 to U-Boot") Signed-off-by: Christophe Leroy christophe.leroy@csgroup.eu

---

lib/zlib/inffast.c | 51 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 40 insertions(+), 11 deletions(-)

Both this, and my mostly revert lead to CI failures around compression tests: https://source.denx.de/u-boot/u-boot/-/jobs/859329

diff --git a/lib/zlib/inffast.c b/lib/zlib/inffast.c index 5e2a65ad4d..c4b1d12258 100644 --- a/lib/zlib/inffast.c +++ b/lib/zlib/inffast.c @@ -236,18 +236,47 @@ unsigned start; /* inflate()'s starting value for strm->avail_out */ } } else {

•     unsigned short *sout;
  

•     unsigned long loops;
  

•                from = out - dist;          /* copy direct from output */
  

•                do {                        /* minimum length is three */
  

•                    *out++ = *from++;
  

•                    *out++ = *from++;
  

•                    *out++ = *from++;
  

•                    len -= 3;
  

•                } while (len > 2);
  

•                if (len) {
  

•                    *out++ = *from++;
  

•                    if (len > 1)
  

•                        *out++ = *from++;
  

•                }
  

•                /* minimum length is three */
  

•     /* Align out addr */
  

•     if (!((long)(out - 1) & 1)) {
  

• 	*out++ = *from++;
  

• 	len--;
  

•     }
  

•     sout = (unsigned short *)out;
  

•     if (dist > 2 ) {
  

• 	unsigned short *sfrom;
  

• 	sfrom = (unsigned short *)from;
  

• 	loops = len >> 1;
  

• 	do
  

• 	    *sout++ = get_unaligned(++sfrom);
  

• 	while (--loops);
  

• 	out = (unsigned char *)sout;
  

• 	from = (unsigned char *)sfrom;
  

•     } else { /* dist == 1 or dist == 2 */
  

• 	unsigned short pat16;
  

• 	pat16 = *(sout-2);
  

• 	if (dist == 1)
  

+#if defined(__BIG_ENDIAN)

• 	    pat16 = (pat16 & 0xff) | ((pat16 & 0xff ) << 8);
  

+#elif defined(__LITTLE_ENDIAN)

• 	    pat16 = (pat16 & 0xff00) | ((pat16 & 0xff00 ) >> 8);
  

+#else +#error __BIG_ENDIAN nor __LITTLE_ENDIAN is defined +#endif

• 	loops = len >> 1;
  

• 	do
  

• 	    *sout++ = pat16;
  

• 	while (--loops);
  

• 	out = (unsigned char *)sout;
  

•     }
  

•     if (len & 1)
  

• 	*out++ = *from++;
          }
      }
      else if ((op & 64) == 0) {          /* 2nd level distance code */
  

-- 2.44.0