[Buildroot] [PATCH 1/1] fs/common.mk: enable multithreaded xz compression

[Buildroot] [PATCH 1/1] fs/common.mk: enable multithreaded xz compression

[james.hilliard1 at gmail.com]

From: James Hilliard <james.hilliard1@gmail.com>

xz help indicates only 1 thread is used unless we set threads:
-T, --threads=NUM   use at most NUM threads; the default is 1; set to 0
                    to use as many threads as there are processor cores

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 fs/common.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/common.mk b/fs/common.mk
index a560417..6a57121 100644
--- a/fs/common.mk
+++ b/fs/common.mk
@@ -106,7 +106,7 @@ endif
 ifeq ($$(BR2_TARGET_ROOTFS_$(2)_XZ),y)
 ROOTFS_$(2)_DEPENDENCIES += host-xz
 ROOTFS_$(2)_COMPRESS_EXT = .xz
-ROOTFS_$(2)_COMPRESS_CMD = xz -9 -C crc32 -c
+ROOTFS_$(2)_COMPRESS_CMD = xz -T 0 -9 -C crc32 -c
 endif
 
 $$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): ROOTFS=$(2)
-- 
2.7.4

[Buildroot] [PATCH 1/1] fs/common.mk: enable multithreaded xz compression

[Matthew Weber]

James,


On Sun, Jan 20, 2019 at 8:08 PM <james.hilliard1@gmail.com> wrote:
>
> From: James Hilliard <james.hilliard1@gmail.com>
>
> xz help indicates only 1 thread is used unless we set threads:
> -T, --threads=NUM   use at most NUM threads; the default is 1; set to 0
>                     to use as many threads as there are processor cores
>

It's good to see xz now has support, but I assume there is a minimum
version?  We ran into this outside of Builroot on a project where we
conditionally used the parallel "pxz" tool when available, as "xz"
didn't have the -T support consistently in distros yet.

> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---
>  fs/common.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/common.mk b/fs/common.mk
> index a560417..6a57121 100644
> --- a/fs/common.mk
> +++ b/fs/common.mk
> @@ -106,7 +106,7 @@ endif
>  ifeq ($$(BR2_TARGET_ROOTFS_$(2)_XZ),y)
>  ROOTFS_$(2)_DEPENDENCIES += host-xz
>  ROOTFS_$(2)_COMPRESS_EXT = .xz
> -ROOTFS_$(2)_COMPRESS_CMD = xz -9 -C crc32 -c
> +ROOTFS_$(2)_COMPRESS_CMD = xz -T 0 -9 -C crc32 -c
>  endif
>
>  $$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): ROOTFS=$(2)
> --
> 2.7.4
>

Thanks for sending the patch!
Matt

[Buildroot] [PATCH 1/1] fs/common.mk: enable multithreaded xz compression

[James Hilliard]

On Mon, Jan 21, 2019 at 7:02 AM Matthew Weber <matthew.weber@collins.com> wrote:
>
> James,
>
>
> On Sun, Jan 20, 2019 at 8:08 PM <james.hilliard1@gmail.com> wrote:
> >
> > From: James Hilliard <james.hilliard1@gmail.com>
> >
> > xz help indicates only 1 thread is used unless we set threads:
> > -T, --threads=NUM   use at most NUM threads; the default is 1; set to 0
> >                     to use as many threads as there are processor cores
> >
>
> It's good to see xz now has support, but I assume there is a minimum
> version?  We ran into this outside of Builroot on a project where we
> conditionally used the parallel "pxz" tool when available, as "xz"
> didn't have the -T support consistently in distros yet.
I thought it was using the version compiled by buildroot.
Isn't that what host-xz is being used for?
>
> > Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> > ---
> >  fs/common.mk | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/common.mk b/fs/common.mk
> > index a560417..6a57121 100644
> > --- a/fs/common.mk
> > +++ b/fs/common.mk
> > @@ -106,7 +106,7 @@ endif
> >  ifeq ($$(BR2_TARGET_ROOTFS_$(2)_XZ),y)
> >  ROOTFS_$(2)_DEPENDENCIES += host-xz
> >  ROOTFS_$(2)_COMPRESS_EXT = .xz
> > -ROOTFS_$(2)_COMPRESS_CMD = xz -9 -C crc32 -c
> > +ROOTFS_$(2)_COMPRESS_CMD = xz -T 0 -9 -C crc32 -c
> >  endif
> >
> >  $$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): ROOTFS=$(2)
> > --
> > 2.7.4
> >
>
> Thanks for sending the patch!
> Matt

[Buildroot] [External] Re: [PATCH 1/1] fs/common.mk: enable multithreaded xz compression

[Matthew Weber]

On Mon, Jan 21, 2019 at 8:05 AM James Hilliard
<james.hilliard1@gmail.com> wrote:
>
> On Mon, Jan 21, 2019 at 7:02 AM Matthew Weber <matthew.weber@collins.com> wrote:
> >
> > James,
> >
> >
> > On Sun, Jan 20, 2019 at 8:08 PM <james.hilliard1@gmail.com> wrote:
> > >
> > > From: James Hilliard <james.hilliard1@gmail.com>
> > >
> > > xz help indicates only 1 thread is used unless we set threads:
> > > -T, --threads=NUM   use at most NUM threads; the default is 1; set to 0
> > >                     to use as many threads as there are processor cores
> > >
> >
> > It's good to see xz now has support, but I assume there is a minimum
> > version?  We ran into this outside of Builroot on a project where we
> > conditionally used the parallel "pxz" tool when available, as "xz"
> > didn't have the -T support consistently in distros yet.
> I thought it was using the version compiled by buildroot.
> Isn't that what host-xz is being used for?

Ah, true.  I had glanced at the dependency checking scripts but we
always built it based on the fs/common.mk dependency on host-xz so
that is correct.

One other comment would be that "0" wouldn't be ideal on a shared
machine.  Suggest limiting to $(PARALLEL_JOBS) which is used in other
filesystem creation (grep fs folder for an example in iso9660 or
squashfs).

Reviewed-by: Matthew Weber <matthew.weber@rockwellcollins.com>

[Buildroot] [PATCH 1/1] fs/common.mk: enable multithreaded xz compression

[Peter Korsgaard]

>>>>> "James" == James Hilliard <james.hilliard1@gmail.com> writes:

 > On Mon, Jan 21, 2019 at 7:02 AM Matthew Weber <matthew.weber@collins.com> wrote:
 >> 
 >> James,
 >> 
 >> 
 >> On Sun, Jan 20, 2019 at 8:08 PM <james.hilliard1@gmail.com> wrote:
 >> >
 >> > From: James Hilliard <james.hilliard1@gmail.com>
 >> >
 >> > xz help indicates only 1 thread is used unless we set threads:
 >> > -T, --threads=NUM   use at most NUM threads; the default is 1; set to 0
 >> >                     to use as many threads as there are processor cores
 >> >
 >> 
 >> It's good to see xz now has support, but I assume there is a minimum
 >> version?  We ran into this outside of Builroot on a project where we
 >> conditionally used the parallel "pxz" tool when available, as "xz"
 >> didn't have the -T support consistently in distros yet.

> I thought it was using the version compiled by buildroot.
> Isn't that what host-xz is being used for?

Yes, correct. Do you know if the -T 0 option generates identical output
as -T 1? Otherwise we may want to disable it for reproducible builds.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 1/1] fs/common.mk: enable multithreaded xz compression

[James Hilliard]

On Mon, Jan 21, 2019 at 10:39 AM Peter Korsgaard <peter@korsgaard.com> wrote:
>
> >>>>> "James" == James Hilliard <james.hilliard1@gmail.com> writes:
>
>  > On Mon, Jan 21, 2019 at 7:02 AM Matthew Weber <matthew.weber@collins.com> wrote:
>  >>
>  >> James,
>  >>
>  >>
>  >> On Sun, Jan 20, 2019 at 8:08 PM <james.hilliard1@gmail.com> wrote:
>  >> >
>  >> > From: James Hilliard <james.hilliard1@gmail.com>
>  >> >
>  >> > xz help indicates only 1 thread is used unless we set threads:
>  >> > -T, --threads=NUM   use at most NUM threads; the default is 1; set to 0
>  >> >                     to use as many threads as there are processor cores
>  >> >
>  >>
>  >> It's good to see xz now has support, but I assume there is a minimum
>  >> version?  We ran into this outside of Builroot on a project where we
>  >> conditionally used the parallel "pxz" tool when available, as "xz"
>  >> didn't have the -T support consistently in distros yet.
>
> > I thought it was using the version compiled by buildroot.
> > Isn't that what host-xz is being used for?
>
> Yes, correct. Do you know if the -T 0 option generates identical output
> as -T 1? Otherwise we may want to disable it for reproducible builds.
I think it doesn't generate identical output, I wonder if it would be
a good idea to make all these compression flags configurable.
One issue I'm running across is that my test builds take at least 5
minutes or so to compress, having a way to set different xz flags
between test and production builds would be handy.
>
> --
> Bye, Peter Korsgaard

[Buildroot] [PATCH 1/1] fs/common.mk: enable multithreaded xz compression

[Thomas Petazzoni]

Hello,

On Mon, 21 Jan 2019 18:39:24 +0100, Peter Korsgaard wrote:

> > I thought it was using the version compiled by buildroot.
> > Isn't that what host-xz is being used for?  
> 
> Yes, correct. Do you know if the -T 0 option generates identical output
> as -T 1? Otherwise we may want to disable it for reproducible builds.

Does it matter than the output of -T 0 and -T 1 are identical ?

What matters for BR2_REPRODUCIBLE=y is if the same input, with -T 0,
always provide the exact identical output.

Am I missing something here ?

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 1/1] fs/common.mk: enable multithreaded xz compression

[James Hilliard]

On Sat, Jan 26, 2019 at 7:06 AM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello,
>
> On Mon, 21 Jan 2019 18:39:24 +0100, Peter Korsgaard wrote:
>
> > > I thought it was using the version compiled by buildroot.
> > > Isn't that what host-xz is being used for?
> >
> > Yes, correct. Do you know if the -T 0 option generates identical output
> > as -T 1? Otherwise we may want to disable it for reproducible builds.
>
> Does it matter than the output of -T 0 and -T 1 are identical ?
>
> What matters for BR2_REPRODUCIBLE=y is if the same input, with -T 0,
> always provide the exact identical output.
My assumption is that -T 0 may be inconsistent due to the autodetection.
>
> Am I missing something here ?
>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

[Buildroot] [PATCH 1/1] fs/common.mk: enable multithreaded xz compression

[Peter Korsgaard]

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes:

 > Hello,
 > On Mon, 21 Jan 2019 18:39:24 +0100, Peter Korsgaard wrote:

 >> > I thought it was using the version compiled by buildroot.
 >> > Isn't that what host-xz is being used for?  
 >> 
 >> Yes, correct. Do you know if the -T 0 option generates identical output
 >> as -T 1? Otherwise we may want to disable it for reproducible builds.

 > Does it matter than the output of -T 0 and -T 1 are identical ?

It would cause an extra change in output between buildroot versions
using -T 0 and ones using -T 1 - But OK, probably that isn't really
important given all the other changes.

What I really meant was if the -T 1 option affected the output format,
and if so, if the difference involved any tradeoff (E.G. perhaps -T 1
compresses slightly worse than -T 0) and if repeated runs with -T 1
would generate the same output (on same machine or on another machine
with different number of threads).

 > What matters for BR2_REPRODUCIBLE=y is if the same input, with -T 0,
 > always provide the exact identical output.

With the new patch that only enables -T 1 for !reproducible builds,
that is correct.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 1/1] fs/common.mk: enable multithreaded xz compression

[James Hilliard]

On Sun, Jan 27, 2019 at 3:06 PM Peter Korsgaard <peter@korsgaard.com> wrote:
>
> >>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes:
>
>  > Hello,
>  > On Mon, 21 Jan 2019 18:39:24 +0100, Peter Korsgaard wrote:
>
>  >> > I thought it was using the version compiled by buildroot.
>  >> > Isn't that what host-xz is being used for?
>  >>
>  >> Yes, correct. Do you know if the -T 0 option generates identical output
>  >> as -T 1? Otherwise we may want to disable it for reproducible builds.
>
>  > Does it matter than the output of -T 0 and -T 1 are identical ?
>
> It would cause an extra change in output between buildroot versions
> using -T 0 and ones using -T 1 - But OK, probably that isn't really
> important given all the other changes.
>
> What I really meant was if the -T 1 option affected the output format,
> and if so, if the difference involved any tradeoff (E.G. perhaps -T 1
> compresses slightly worse than -T 0) and if repeated runs with -T 1
> would generate the same output (on same machine or on another machine
> with different number of threads).
The single threaded -T 1 should produce the best compression quality,
although it's a negligible difference.
From my understanding that is because single threaded compression
doesn't need to split up the file being compressed into separate
blocks.
>
>  > What matters for BR2_REPRODUCIBLE=y is if the same input, with -T 0,
>  > always provide the exact identical output.
>
> With the new patch that only enables -T 1 for !reproducible builds,
> that is correct.
>
> --
> Bye, Peter Korsgaard

[Buildroot] [PATCH 1/1] fs/common.mk: enable multithreaded xz compression

[Peter Korsgaard]

>>>>> "Peter" == Peter Korsgaard <peter@korsgaard.com> writes:

 >> Does it matter than the output of -T 0 and -T 1 are identical ?

 > It would cause an extra change in output between buildroot versions
 > using -T 0 and ones using -T 1 - But OK, probably that isn't really
 > important given all the other changes.

 > What I really meant was if the -T 1 option affected the output format,
 > and if so, if the difference involved any tradeoff (E.G. perhaps -T 1
 > compresses slightly worse than -T 0) and if repeated runs with -T 1
 > would generate the same output (on same machine or on another machine
 > with different number of threads).

Sorry, got -T 1 and -T 0 mixed up, I meant -T 0 here.

 >> What matters for BR2_REPRODUCIBLE=y is if the same input, with -T 0,
 >> always provide the exact identical output.

 > With the new patch that only enables -T 1 for !reproducible builds,
 > that is correct.

The assumption is that this is not true, especially between systems with
different number of CPUs.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 1/1] fs/common.mk: enable multithreaded xz compression

[Thomas Petazzoni]

On Sun, 27 Jan 2019 23:06:14 +0100
Peter Korsgaard <peter@korsgaard.com> wrote:

>  > Does it matter than the output of -T 0 and -T 1 are identical ?  
> 
> It would cause an extra change in output between buildroot versions
> using -T 0 and ones using -T 1 - But OK, probably that isn't really
> important given all the other changes.

Well, BR2_REPRODUCIBLE is definitely not about guaranteeing identical
output between Buildroot versions. How could this even work, since we
are bumping the version of dozens/hundreds of packages ?

BR2_REPRODUCIBLE is about given a fixed Buildroot version and
configuration, be able to generate a binary-identical result.

>  > What matters for BR2_REPRODUCIBLE=y is if the same input, with -T 0,
>  > always provide the exact identical output.  
> 
> With the new patch that only enables -T 1 for !reproducible builds,
> that is correct.

I thought -T 0 was the problematic thing as it enabled automatic
parallelization, while -T 1 explicitly disabled parallelization.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 1/1] fs/common.mk: enable multithreaded xz compression

[James Hilliard]

On Mon, Jan 28, 2019 at 1:23 AM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> On Sun, 27 Jan 2019 23:06:14 +0100
> Peter Korsgaard <peter@korsgaard.com> wrote:
>
> >  > Does it matter than the output of -T 0 and -T 1 are identical ?
> >
> > It would cause an extra change in output between buildroot versions
> > using -T 0 and ones using -T 1 - But OK, probably that isn't really
> > important given all the other changes.
>
> Well, BR2_REPRODUCIBLE is definitely not about guaranteeing identical
> output between Buildroot versions. How could this even work, since we
> are bumping the version of dozens/hundreds of packages ?
>
> BR2_REPRODUCIBLE is about given a fixed Buildroot version and
> configuration, be able to generate a binary-identical result.
>
> >  > What matters for BR2_REPRODUCIBLE=y is if the same input, with -T 0,
> >  > always provide the exact identical output.
> >
> > With the new patch that only enables -T 1 for !reproducible builds,
> > that is correct.
>
> I thought -T 0 was the problematic thing as it enabled automatic
> parallelization, while -T 1 explicitly disabled parallelization.
Right, -T 1 is the default.
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

[Buildroot] [PATCH 1/1] fs/common.mk: enable multithreaded xz compression

[Peter Korsgaard]

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes:

 > On Sun, 27 Jan 2019 23:06:14 +0100
 > Peter Korsgaard <peter@korsgaard.com> wrote:

 >> > Does it matter than the output of -T 0 and -T 1 are identical ?  
 >> 
 >> It would cause an extra change in output between buildroot versions
 >> using -T 0 and ones using -T 1 - But OK, probably that isn't really
 >> important given all the other changes.

 > Well, BR2_REPRODUCIBLE is definitely not about guaranteeing identical
 > output between Buildroot versions. How could this even work, since we
 > are bumping the version of dozens/hundreds of packages ?

 > BR2_REPRODUCIBLE is about given a fixed Buildroot version and
 > configuration, be able to generate a binary-identical result.

Correct, hence my note "isn't really important".


 >> > What matters for BR2_REPRODUCIBLE=y is if the same input, with -T 0,
 >> > always provide the exact identical output.  
 >> 
 >> With the new patch that only enables -T 1 for !reproducible builds,
 >> that is correct.

 > I thought -T 0 was the problematic thing as it enabled automatic
 > parallelization, while -T 1 explicitly disabled parallelization.

Sorry, I got T 0/1 mixed up in my mail, like I explained in the
followup.

-- 
Bye, Peter Korsgaard