From: Nick Terrell <terrelln@fb.com>
To: "dsterba@suse.cz" <dsterba@suse.cz>
Cc: Adam Borowski <kilobyte@angband.pl>,
Kernel Team <Kernel-team@fb.com>, Chris Mason <clm@fb.com>,
Yann Collet <cyan@fb.com>,
"squashfs-devel@lists.sourceforge.net"
<squashfs-devel@lists.sourceforge.net>,
"linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] lib/zstd: use div_u64() to let it build on 32-bit
Date: Wed, 28 Jun 2017 05:29:25 +0000 [thread overview]
Message-ID: <0237381B-1B0B-4666-BA07-ABCEBDD492AE@fb.com> (raw)
In-Reply-To: <20170627125730.GV2866@suse.cz>
> Please don't top post.
Sorry about that.
> Which function needs 1KB of stack space? That's quite a lot.
FSE_buildCTable_wksp(), FSE_compress_wksp(), and HUF_readDTableX4()
required over 1 KB of stack space.
> I can see in [1] that there are some on-stack buffers replaced by
> pointers to the workspace. That's good, but I would like to know if
> there's any hidden gem that grags the precious stack space.
I've been hunting down functions that use up the most stack trace and
replacing buffers with pointers to the workspace. I compiled the code
with -Wframe-larger-than=512 and reduced the stack usage of all offending
functions. In the next version of the patch, no function uses more than
400 B of stack space. We'll be porting the changes back upstream as well.
> Hm, I'd suggest to create a version optimized for kernel, eg. expecting
> that 4+ GB buffer will never be used and you can use the most fittin in
> type. This should affect only the function signatures, not the
> algorithm implementation, so porting future zstd changes should be
> straightforward.
If the functions were exposed, then I would agree 100%. However, since
these are internal functions, and the rest of zstd uses size_t to represent
buffer sizes, I think it would be awkward to change just FSE/HUF functions.
I also prefer size_t because it is friendlier to the optimizer, especially
the loop optimizer, since the compiler doesn't have to worry about unsigned
overflow.
On a related note, zstd performs automatic optimizations to improve
compression speed and reduce memory usage when given small sources, which
is the common case in the kernel.
next prev parent reply other threads:[~2017-06-28 5:29 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-22 22:01 [PATCH 1/4] lib: Add xxhash module Nick Terrell
2017-06-22 22:01 ` [PATCH 2/4] lib: Add zstd modules Nick Terrell
2017-06-22 22:01 ` [PATCH 3/4] btrfs: Add zstd support Nick Terrell
2017-06-25 15:02 ` kbuild test robot
2017-06-25 19:03 ` kbuild test robot
2017-06-25 21:30 ` Adam Borowski
2017-06-26 12:12 ` David Sterba
2017-06-26 16:54 ` Nick Terrell
2017-06-27 4:18 ` [PATCH] lib/zstd: use div_u64() to let it build on 32-bit Adam Borowski
2017-06-27 5:27 ` Nick Terrell
2017-06-27 12:57 ` David Sterba
2017-06-28 5:29 ` Nick Terrell [this message]
2017-06-29 1:02 ` Adam Borowski
2017-06-29 3:01 ` [PATCH] btrfs: Keep one more workspace around Nick Terrell
2017-06-29 13:59 ` David Sterba
2017-06-22 22:01 ` [PATCH 4/4] squashfs: Add zstd support Nick Terrell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0237381B-1B0B-4666-BA07-ABCEBDD492AE@fb.com \
--to=terrelln@fb.com \
--cc=Kernel-team@fb.com \
--cc=clm@fb.com \
--cc=cyan@fb.com \
--cc=dsterba@suse.cz \
--cc=kilobyte@angband.pl \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=squashfs-devel@lists.sourceforge.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox