Merge lp://qastaging/~mwhudson/livecd-rootfs/live-server-gzip-initramfs into lp://qastaging/livecd-rootfs

Proposed by Michael Hudson-Doyle
Status: Merged
Merged at revision: 1657
Proposed branch: lp://qastaging/~mwhudson/livecd-rootfs/live-server-gzip-initramfs
Merge into: lp://qastaging/livecd-rootfs
Diff against target: 56 lines (+14/-3)
2 files modified
debian/changelog (+7/-0)
live-build/auto/config (+7/-3)
To merge this branch: bzr merge lp://qastaging/~mwhudson/livecd-rootfs/live-server-gzip-initramfs
Reviewer Review Type Date Requested Status
Steve Langasek Approve
Balint Reczey Pending
Review via email: mp+341326@code.qastaging.launchpad.net

Description of the change

I'm fairly sure this will make the boot faster in KVM at least but does anyone have any ideas how to quantify it? Will it be replicated on real hardware?

To post a comment you must log in.
1640. By Michael Hudson-Doyle

Set INITRAMFS_COMPRESSION to gzip for live-server builds to speed up the
boot. (LP: #1750873)

Revision history for this message
Steve Langasek (vorlon) wrote :

Tagging Balint for a review, to speak to / quantify the tradeoffs here between gzip and lzma (compressed size + time to read initramfs from bootloader, vs. time to decompress in kernel).

There's no support for this yet in livecd-rootfs + live-build, but this seems like an ideal use case for lz4 instead - compressed initrd created once server-side, and uncompressed many times
on each boot of the live image. But we're still a way out from being able to make such a change and shouldn't block on it for bionic.

Also, aligning with my previous feedback to @rcj, we should really not be shadowing the live-build default compression here, but instead ensure $INITRAMFS_COMPRESSION is unset for cases where live-build defaults (currently gzip; future, lz4?) DTRT.

review: Needs Fixing
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

> Tagging Balint for a review, to speak to / quantify the tradeoffs here between
> gzip and lzma (compressed size + time to read initramfs from bootloader, vs.
> time to decompress in kernel).

Thanks.

> There's no support for this yet in livecd-rootfs + live-build

Or mkinitramfs, given that you need to pass -l to lz4 when compressing to get the kernel to understand the initrd.lz4.

> , but this seems
> like an ideal use case for lz4 instead - compressed initrd created once
> server-side, and uncompressed many times
> on each boot of the live image. But we're still a way out from being able to
> make such a change and shouldn't block on it for bionic.

I did try lz4 in my playing around and it did not compress as well as gzip:

-rw-r--r-- 1 root root 148M Mar 12 12:59 initrd
-rw-r--r-- 1 root root 55M Mar 12 13:00 initrd.gz.3
-rw-r--r-- 1 root root 52M Mar 12 13:00 initrd.gz.6
-rw-r--r-- 1 root root 52M Mar 12 13:00 initrd.gz.9
-r--r--r-- 1 root root 33M Mar 11 20:36 initrd.lz
-rw-r--r-- 1 root root 61M Mar 12 13:01 initrd.lz4.3
-rw-r--r-- 1 root root 60M Mar 12 13:01 initrd.lz4.6
-rw-r--r-- 1 root root 60M Mar 12 13:00 initrd.lz4.9

It may have been faster to decompress but it was 0.7s for gzip, 0.4s for lz4 vs 5.2s for lzma sort of numbers.

Getting support for lz4 initrds would seem to involve changing a lot of things (e.g. initrd_suffix in debian-cd) and generally doesn't seem worth it to me at this point in the cycle.

Does the kernel support zstd initrds yet? :)

> Also, aligning with my previous feedback to @rcj, we should really not be
> shadowing the live-build default compression here, but instead ensure
> $INITRAMFS_COMPRESSION is unset for cases where live-build defaults (currently
> gzip; future, lz4?) DTRT.

So you'd prefer something along the lines of this:

=== modified file 'live-build/auto/config'
--- live-build/auto/config 2018-03-13 08:37:06 +0000
+++ live-build/auto/config 2018-03-13 20:31:46 +0000
@@ -818,7 +818,7 @@
  ${KERNEL_FLAVOURS:+--linux-flavours "$KERNEL_FLAVOURS"} \
  --initsystem none \
  --bootloader "$BOOTLOADER" \
- --initramfs-compression "${INITRAMFS_COMPRESSION:-lzma}" \
+ ${INITRAMFS_COMPRESSION:+--initramfs-compression "$INITRAMFS_COMPRESSION}" \
  --cache false \
  ${BOOTAPPEND_LIVE:+--bootappend-live "$BOOTAPPEND_LIVE"} \
  $OPTS \

(live-build appears to default to gzip by my reading)? That makes sense but is it safe to land at this point of the cycle?

Revision history for this message
Steve Langasek (vorlon) wrote :

On Tue, Mar 13, 2018 at 08:38:21PM -0000, Michael Hudson-Doyle wrote:
> > Also, aligning with my previous feedback to @rcj, we should really not be
> > shadowing the live-build default compression here, but instead ensure
> > $INITRAMFS_COMPRESSION is unset for cases where live-build defaults (currently
> > gzip; future, lz4?) DTRT.

> So you'd prefer something along the lines of this:

> === modified file 'live-build/auto/config'
> --- live-build/auto/config 2018-03-13 08:37:06 +0000
> +++ live-build/auto/config 2018-03-13 20:31:46 +0000
> @@ -818,7 +818,7 @@
> ${KERNEL_FLAVOURS:+--linux-flavours "$KERNEL_FLAVOURS"} \
> --initsystem none \
> --bootloader "$BOOTLOADER" \
> - --initramfs-compression "${INITRAMFS_COMPRESSION:-lzma}" \
> + ${INITRAMFS_COMPRESSION:+--initramfs-compression "$INITRAMFS_COMPRESSION}" \
> --cache false \
> ${BOOTAPPEND_LIVE:+--bootappend-live "$BOOTAPPEND_LIVE"} \
> $OPTS \

Yes, except that you also need to fix up the other image flavor cases to set
lzma where it's currently set.

> (live-build appears to default to gzip by my reading)? That makes sense
> but is it safe to land at this point of the cycle?

Provided we have appropriate code review wrt the above to ensure we are only
changing the value for the server live CD and not accidentally changing it
for any of the others, I think this is fine to land.

--
Steve Langasek Give me a lever long enough and a Free OS
Debian Developer to set it on, and I can move the world.
Ubuntu Developer http://www.debian.org/
<email address hidden> <email address hidden>

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

> On Tue, Mar 13, 2018 at 08:38:21PM -0000, Michael Hudson-Doyle wrote:
> > > Also, aligning with my previous feedback to @rcj, we should really not be
> > > shadowing the live-build default compression here, but instead ensure
> > > $INITRAMFS_COMPRESSION is unset for cases where live-build defaults
> (currently
> > > gzip; future, lz4?) DTRT.
>
> > So you'd prefer something along the lines of this:
>
> > === modified file 'live-build/auto/config'
> > --- live-build/auto/config 2018-03-13 08:37:06 +0000
> > +++ live-build/auto/config 2018-03-13 20:31:46 +0000
> > @@ -818,7 +818,7 @@
> > ${KERNEL_FLAVOURS:+--linux-flavours "$KERNEL_FLAVOURS"} \
> > --initsystem none \
> > --bootloader "$BOOTLOADER" \
> > - --initramfs-compression "${INITRAMFS_COMPRESSION:-lzma}" \
> > + ${INITRAMFS_COMPRESSION:+--initramfs-compression
> "$INITRAMFS_COMPRESSION}" \
> > --cache false \
> > ${BOOTAPPEND_LIVE:+--bootappend-live "$BOOTAPPEND_LIVE"} \
> > $OPTS \
>
> Yes, except that you also need to fix up the other image flavor cases to set
> lzma where it's currently set.

Currently not set, I assume you mean? The case statement I am editing in the current MP has 22 cases, only 2 (or 3 in my MP) set INITRAMFS_COMPRESSION today. It's obviously not _hard_ to add INITRAMFS_COMPRESSION=lzma to the other 19 but is that what you want? I can do it if so...

> > (live-build appears to default to gzip by my reading)? That makes sense
> > but is it safe to land at this point of the cycle?
>
> Provided we have appropriate code review wrt the above to ensure we are only
> changing the value for the server live CD and not accidentally changing it
> for any of the others, I think this is fine to land.
>
> --
> Steve Langasek Give me a lever long enough and a Free OS
> Debian Developer to set it on, and I can move the world.
> Ubuntu Developer http://www.debian.org/
> <email address hidden> <email address hidden>

1641. By Michael Hudson-Doyle

Make lzma initramfs compression opt-in rather than opt-out, and do not
opt-in to it for live-server builds to speed up the boot. (LP: #1750873)

Revision history for this message
Steve Langasek (vorlon) wrote :

On Wed, Mar 14, 2018 at 02:50:23AM -0000, Michael Hudson-Doyle wrote:
> > Yes, except that you also need to fix up the other image flavor cases to set
> > lzma where it's currently set.

> Currently not set, I assume you mean?

Yes, precisely :)

> The case statement I am editing in the current MP has 22 cases, only 2 (or
> 3 in my MP) set INITRAMFS_COMPRESSION today. It's obviously not _hard_ to
> add INITRAMFS_COMPRESSION=lzma to the other 19 but is that what you want?
> I can do it if so...

I think it's probably appropriate to add this as a separate case statement
to avoid *quite* so much code duplication; but yes, we should explicitly
call out the products individually where we want to use lzma.

1642. By Michael Hudson-Doyle

reduce duplication somewhat

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

> I think it's probably appropriate to add this as a separate case statement
> to avoid *quite* so much code duplication; but yes, we should explicitly
> call out the products individually where we want to use lzma.

Please have another look.

Revision history for this message
Steve Langasek (vorlon) wrote :

One comment, otherwise LGTM

review: Needs Fixing
1643. By Michael Hudson-Doyle

do not set INITRAMFS_COMPRESSION for projects that end up passing --initramfs=none to lb config

Revision history for this message
Steve Langasek (vorlon) :
review: Approve
1644. By Michael Hudson-Doyle

merge trunk

Revision history for this message
Balint Reczey (rbalint) wrote :

Zstd initrd support is not there yet in the kernel thus it is not easily backportable to older LTS releases. While lz4 compresses worse the speedup compared to gz is substantial and not all tools have to be changed before 18.04 GA.

I took @vorlon's "does not seem urgent" [1] as a soft-block before 18.04 GA but I would be happier to just add lz4 support and we can provide slightly bigger but faster-booting images.

[1] https://lists.ubuntu.com/archives/ubuntu-devel/2018-March/040265.html

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches