lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 16 Apr 2018 19:34:29 +0000
From:   Yann Collet <cyan@...com>
To:     "maninder1.s@...sung.com" <maninder1.s@...sung.com>,
        Vaneet Narang <v.narang@...sung.com>,
        Nick Terrell <terrelln@...com>,
        Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        "herbert@...dor.apana.org.au" <herbert@...dor.apana.org.au>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "minchan@...nel.org" <minchan@...nel.org>,
        "ngupta@...are.org" <ngupta@...are.org>,
        Kees Cook <keescook@...omium.org>,
        "anton@...msg.org" <anton@...msg.org>,
        "ccross@...roid.com" <ccross@...roid.com>,
        "tony.luck@...el.com" <tony.luck@...el.com>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "colin.king@...onical.com" <colin.king@...onical.com>,
        "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        PANKAJ MISHRA <pankaj.m@...sung.com>,
        "AMIT SAHRAWAT" <a.sahrawat@...sung.com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
Subject: Re: [PATCH 0/1] cover-letter/lz4: Implement lz4 with dynamic offset
 length.

Hi Singh

I don't have any strong opinion on this topic.

You made your case clear: 
your variant trades a little bit of speed for a little bit more compression ratio.
In the context of zram, it makes sense, and I would expect it to work, as advertised in your benchmark results.
(disclaimer: I haven't reproduced these results, just, they look reasonable to me, I have no reason to doubt them).

So, the issue is less about performance, than about code complexity.

As mentioned, this is an incompatible variant.
So, it requires its own entry point, and preferably its own code path
(even if it's heavily duplicated,
mixing it with regular lz4 source code, as proposed in the patch, will be bad for maintenance, 
and can negatively impact regular lz4 usage, outside of zram).

So that's basically the "cost" of adding this option.

Is it worth it?
Well, this is completely outside of my responsibility area, so I really can't tell.
You'll have to convince people in charge that the gains are worth their complexity,
since _they_ will inherit the duty to keep the system working through its future evolutions.
At a minimum, you are targeting maintainers of zram and the crypto interface.
For this topic, they are the right people to talk to.


´╗┐On 4/16/18, 04:09, "Maninder Singh" <maninder1.s@...sung.com> wrote:

    
     Hello Nick/ Yann,
    
    Any inputs regarding LZ4 dyn results & lz4 dyn approach.
    
    >Hello Nick/Sergey,
    > 
    >Any suggestion or comments, so that we can change code and resend the patch?
    > 
    >> Hi Nick / Sergey,
    >> 
    >> 
    >> We have compared LZ4 Dyn with Original LZ4 using some samples of realtime application data(4Kb)
    >> compressed/decompressed by ZRAM. For comparison we have used lzbench (https://github.com/inikep/lzbench)
    >> we have implemented dedicated LZ4 Dyn API & kept last literal length as 6 to avoid overhead 
    >> of checks. It seems in average case there is a saving of 3~4% in compression ratio with almost same compression
    >> speed and minor loss in decompression speed (~50MB/s) when compared with LZ4.
    >> 
    >> Comparison of Lz4 Dyn with LZO1x is also done as LZO1x is default compressor of ZRAM.
    >> 
    >> Original LZ4:
    >> sh-3.2# ./lzbench  -r  -elz4  data/
    >> lzbench 1.7.3 (32-bit Linux)   Assembled by P.Skibinski
    >> Compressor name         Compress. Decompress. Compr. size  Ratio Filename
    >> memcpy                   2205 MB/s  2217 MB/s        4096 100.00 data//data_1
    >> lz4 1.8.0                 216 MB/s   761 MB/s        2433  59.40 data//data_1
    >> lz4 1.8.0                 269 MB/s   877 MB/s        1873  45.73 data//data_2
    >> lz4 1.8.0                 238 MB/s   575 MB/s        2060  50.29 data//data_3
    >> lz4 1.8.0                 321 MB/s  1015 MB/s        1464  35.74 data//data_4
    >> lz4 1.8.0                 464 MB/s  1090 MB/s         713  17.41 data//data_5
    >> lz4 1.8.0                 296 MB/s   956 MB/s        1597  38.99 data//data_6
    >> lz4 1.8.0                 338 MB/s   994 MB/s        2238  54.64 data//data_7
    >> lz4 1.8.0                 705 MB/s  1172 MB/s         193   4.71 data//data_8
    >> lz4 1.8.0                 404 MB/s  1150 MB/s        1097  26.78 data//data_9
    >> lz4 1.8.0                 216 MB/s   921 MB/s        3183  77.71 data//data_10
    >> lz4 1.8.0                 456 MB/s  1101 MB/s        1011  24.68 data//data_11
    >> lz4 1.8.0                 867 MB/s  1202 MB/s          37   0.90 data//data_12
    >> 
    >> 
    >> LZ4 Dynamic Offet:  
    >> sh-3.2# ./lzbench  -r  -elz4_dyn  data/
    >> lzbench 1.7.3 (32-bit Linux)   Assembled by P.Skibinski
    >> Compressor name         Compress. Decompress. Compr. size  Ratio Filename
    >> memcpy                   2203 MB/s  2218 MB/s        4096 100.00 data//data_1
    >> lz4 1.8.0                 218 MB/s   693 MB/s        2228  54.39 data//data_1
    >> lz4 1.8.0                 273 MB/s   851 MB/s        1739  42.46 data//data_2
    >> lz4 1.8.0                 230 MB/s   526 MB/s        1800  43.95 data//data_3
    >> lz4 1.8.0                 321 MB/s   952 MB/s        1357  33.13 data//data_4
    >> lz4 1.8.0                 470 MB/s  1075 MB/s         664  16.21 data//data_5
    >> lz4 1.8.0                 303 MB/s   964 MB/s        1455  35.52 data//data_6
    >> lz4 1.8.0                 345 MB/s   951 MB/s        2126  51.90 data//data_7
    >> lz4 1.8.0                 744 MB/s  1163 MB/s         177   4.32 data//data_8
    >> lz4 1.8.0                 409 MB/s  1257 MB/s        1033  25.22 data//data_9
    >> lz4 1.8.0                 220 MB/s   857 MB/s        3049  74.44 data//data_10
    >> lz4 1.8.0                 464 MB/s  1105 MB/s         934  22.80 data//data_11
    >> lz4 1.8.0                 874 MB/s  1194 MB/s          36   0.88 data//data_12
    >> 
    >> 
    >> LZ4 Dynamic Offset with 32K data:
    >> sh-3.2# ./lzbench -elz4_dyn data/data32k
    >> lzbench 1.7.3 (32-bit Linux)   Assembled by P.Skibinski
    >> Compressor name         Compress. Decompress. Compr. size  Ratio Filename
    >> memcpy                   5285 MB/s  5283 MB/s       32768 100.00 data/data32k
    >> lz4 1.8.0                 274 MB/s   995 MB/s       13435  41.00 data/data32k
    >> done... (cIters=1 dIters=1 cTime=1.0 dTime=2.0 chunkSize=1706MB cSpeed=0MB)
    >> 
    >> Original LZ4 with 32K data:
    >> sh-3.2# ./lzbench_orig -elz4 data/data32k
    >> lzbench 1.7.3 (32-bit Linux)   Assembled by P.Skibinski
    >> Compressor name         Compress. Decompress. Compr. size  Ratio Filename
    >> memcpy                   4918 MB/s  5108 MB/s       32768 100.00 data/data32k
    >> lz4 1.8.0                 276 MB/s  1045 MB/s       14492  44.23 data/data32k
    >> 
    >> LZO1x with 32K data (Default Compressor for ZRAM): 
    >> sh-3.2# ./lzbench -elzo1x,1 data/data32k
    >> lzbench 1.7.3 (32-bit Linux)   Assembled by P.Skibinski
    >> Compressor name         Compress. Decompress. Compr. size  Ratio Filename
    >> memcpy                   5273 MB/s  5320 MB/s       32768 100.00 data/data32k
    >> lzo1x 2.09 -1             283 MB/s   465 MB/s       14292  43.62 data/data32k
    
    
    Thanks,
    Maninder Singh
     
    

Powered by blists - more mailing lists