From: Nick Terrell <terrelln@fb.com>
To: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
CC: Joonsoo Kim <iamjoonsoo.kim@lge.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "minchan@kernel.org" <minchan@kernel.org>, Yann Collet <cyan@fb.com>
Subject: Re: [PATCH] zram: add zstd to the supported algorithms list
Thread-Topic: [PATCH] zram: add zstd to the supported algorithms list
Thread-Index: AQHTHSCvXWqWlMVfbUKYI//VGek9A6KUPdeA//+XcYCAAHo6gP//jzWA
Date: Fri, 25 Aug 2017 02:09:20 +0000
Message-ID: <08620C31-AD6A-4DDB-ACA6-22243920AE1E@fb.com>
References: <20170824014936.4738-1-sergey.senozhatsky@gmail.com>
 <AA6434F2-6932-436E-B063-B6C0417A553C@contoso.com>
 <20170825004947.GE29701@js1304-P5Q-DELUXE>
 <27EDD68A-61DC-42F1-8422-16B9AB9F0EB3@fb.com>
 <20170825015301.GB743@jagdpanzerIV.localdomain>
In-Reply-To: <20170825015301.GB743@jagdpanzerIV.localdomain>
Accept-Language: en-US
Content-Language: en-US
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="utf-8"
Content-ID: <D2FC00A830CA3D41B44395FF4BACB42F@namprd15.prod.outlook.com>
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-originalarrivaltime: 25 Aug 2017 02:09:20.1212
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR15MB1593
X-OriginatorOrg: fb.com
X-Proofpoint-Spam-Reason: safe
X-FB-Internal: Safe
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-08-25_01:,,
 signatures=0
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from base64 to 8bit by nfs id v7P29YfW020741
Content-Length: 2414
Lines: 61

On 8/24/17, 6:53 PM, "Sergey Senozhatsky" <sergey.senozhatsky.work@gmail.com> wrote:
> On (08/25/17 01:35), Nick Terrell wrote:
> > On 8/24/17, 5:49 PM, "Joonsoo Kim" <iamjoonsoo.kim@lge.com> wrote:
> > > On Thu, Aug 24, 2017 at 09:33:54PM +0000, Nick Terrell wrote:
> > > > On Thu, Aug 24, 2017 at 10:49:36AM +0900, Sergey Senozhatsky wrote:
> > > > > Add ZSTD to the list of supported compression algorithms.
> > > > > 
> > > > > Official benchmarks [1]:
> > > > 
> > > > Awesome! Let me know if you need anything from me.
> > > > 
> > > Hello, Nick.
> > > 
> > > Awesome work!!!
> > > 
> > > Let me ask a question.
> > > Zram compress and decompress a small data (a page) and your github
> > > site says that using predefined dictionary would be helpful in this
> > > situation. However, it seems that compression crypto API for zstd
> > > doesn't use ZSTD_compress_usingDict(). Is there any plan to support
> > > it?
> 
> excellent question, Joonsoo.
> 
> > I think using dictionaries in zram could be very interesting. We could for
> > example, take a random sample of the RAM and use that as the dictionary
> > for compression. E.g. take 32 512B samples from RAM and build a 16 KB
> > dictionary (sizes may vary).
> > 
> > I'm not sure how you would pass a dictionary into the crypto compression
> > API, but I'm sure we can make something work if dictionary compression
> > proves to be beneficial enough.
> 
> a dictionaty pointer can be in `struct zstd_ctx'.

Sorry I'm not super familiar with the crypto API. How would the user
initialize the dictionary in the `struct zstd_ctx'? I guess you could
either expose the struct in some header, or provide a function that
initializes it, but that isn't generic. Does the crypto library already
have a standard way to provide extra information?

> > What data have you, or anyone, used for benchmarking compression ratio and 
> > speed for RAM? Since it is such a specialized application, the standard
> > compression benchmarks aren't very applicable.
> 
> 
> yeah, I thought that zstd uses dicts unconditionally.
> 
> I used my own simple minded test script:
> 	https://github.com/sergey-senozhatsky/zram-perf-test
> 
> it basically invokes fio with a 'static compression buffer', because
> we want to have the exactly same data to be compressed when I compare
> algorithms... I guess I need to improve it, somehow.

I'll check it out, thanks!

> 
> 	-ss
>