On Sun, 2016-12-04 at 16:07:16 +0200, Adrian Bunk wrote:
> On Sun, Dec 04, 2016 at 05:25:49AM +0100, Guillem Jover wrote:
> > On Fri, 2016-12-02 at 10:31:58 +0100, Guillem Jover wrote:
> > > Right, this was reported the other day on IRC by Mattia Rizzolo. The
> > > combination of -Sextreme -z9 and parallel xz makes this use more than
> > > the available address space. I'll change the code to limit based on
> > > memory available. Although as was mentioned on a thread on d-d, those
> > > settings are pretty unfriendly IMO, even more for memory constrained
> > > arches, but oh well. dpkg should never fail to operate on those
> > > conditions.
> >
> > I've got the attached patch now, but I've been unable to test that
> > specific incarnation as I don't have 32-bit machine with many cores.
> > And neither are the i386 nor armhf porter boxes. I've just verified
> > that it does what it is intended by hardcoding the number of threads
> > to 32 and setting the physical memory limit to 2 GiB. And it reduced
> > the threads down to 12 when building one of those font packages.
> Your patch won't solve it - the problem is not physical memory.
>
> The problem is that on 32 bit you cannot use more than 2-3 GB
> (depending on the architecture) in one process.
Right, I'm aware of the distinction, but I guess I was wrongly
assuming the lzma_physmem() function would return (userspace)
addressable physical memory. Checking it now, it seems like not,
as it uses stuff like sysconf(_SC_PHYS_PAGES).
> #845757 was on a mipsel buildd with 4 cores [1] and 8 GB RAM.
Ah, thanks, I'm testing as I write this a new patch I've prepared on
eller.d.o (mipsel) which seems to have the right conditions.
> I don't know how much RAM the amd64/i386 buildds have,
> but I'd guess more than 4 GB...
>
> A hard upper limit somewhere around 1.5 GB on all 32 bit architectures
> (or on all architectures, if that's easier to implement) is required.
The patch now clamps the physical memory to INTPTR_MAX. So that we both
do not exceed the physical memory available nor the addressable limit.
But I should indeed probably also subtract some space for the rest of
the userland that might be running. I'll do that later today.
> (I'd also suggest a MBF for all the packages that mess with the
> compressor for no good reason - just like the ones using bzip2
> this only causes troubles.)
This has been brought up on d-d some time ago. And many of the
packages affected do not even really benefit from such extreme
settings as the dictionary size is bigger than the actual data
to compress. There's a bug in lintian to report an error on this.
Thanks,
Guillem
From f48c09879e6ba91110a2918a3caea5985ef57a3f Mon Sep 17 00:00:00 2001
From: Guillem Jover <guil...@debian.org>
Date: Sun, 4 Dec 2016 02:35:27 +0100
Subject: [PATCH] libdpkg: Decrease xz encoder threads to not exceed memory
limits
---
lib/dpkg/compress.c | 27 ++++++++++++++++++++++++++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/lib/dpkg/compress.c b/lib/dpkg/compress.c
index 2eda658fa..a6dd522ed 100644
--- a/lib/dpkg/compress.c
+++ b/lib/dpkg/compress.c
@@ -529,9 +529,9 @@ filter_xz_init(struct io_lzma *io, lzma_stream *s)
uint32_t preset;
lzma_check check = LZMA_CHECK_CRC64;
#ifdef HAVE_LZMA_MT
+ uint64_t mt_memlimit;
lzma_mt mt_options = {
.flags = 0,
- .threads = sysconf(_SC_NPROCESSORS_ONLN),
.block_size = 0,
.timeout = 0,
.filters = NULL,
@@ -548,6 +548,31 @@ filter_xz_init(struct io_lzma *io, lzma_stream *s)
#ifdef HAVE_LZMA_MT
mt_options.preset = preset;
+
+ /* Initialize the multi-threaded memory limit to half the physical
+ * RAM, or to 128 MiB if we cannot infer the number. */
+ mt_memlimit = lzma_physmem() / 2;
+ if (mt_memlimit == 0)
+ mt_memlimit = 128 * 1024 * 1024;
+ /* Clamp the multi-threaded memory limit to half the addressable
+ * memory on this architecture. */
+ if (mt_memlimit > INTPTR_MAX)
+ mt_memlimit = INTPTR_MAX;
+
+ mt_options.threads = lzma_cputhreads();
+ if (mt_options.threads == 0)
+ mt_options.threads = 1;
+
+ /* Check that we have enough RAM to use the multi-threaded encoder,
+ * and decrease them up to single-threaded to reduce memory usage. */
+ for (; mt_options.threads > 1; mt_options.threads--) {
+ uint64_t mt_memusage;
+
+ mt_memusage = lzma_stream_encoder_mt_memusage(&mt_options);
+ if (mt_memusage < mt_memlimit)
+ break;
+ }
+
ret = lzma_stream_encoder_mt(s, &mt_options);
#else
ret = lzma_easy_encoder(s, preset, check);
--
2.11.0.rc1.160.g51e66c2
