Optimize "install -s" and installworld

[eugen_grosbein.net]

Summary

The command "make installworld" performs multiple invocations of "install -s -S" to install and strip binaries from obj directory to DESTDIR.

Currently, "install -s" behaviour is inefficient for upgrade. First it finds that destination file already exists and copies source file to temporary file. Then it calls strip(1) with name of temporary file as single agrument and our strip(1) creates another temporary file in the /tmp (or TMPDIR) making another copy that is finally copied to DESTDIR third time.

Meantime, strip(1) has an option "-o dst" to specify destination so install(1) is allowed to skip initial copying from obj to DESTDIR. This change makes it do so.

This optimization descreases total amount of data sent to both of /tmp and DESTDIR during "make installworld" for 32% in case of upgrade of 11.4-RELEASE/amd64 to stable/11 r362797 comparing unpatched xinstall.c: 679MB vs. 999MB

Also, this change decreases required amount of free space for /usr partition and may prevent installworld from failing for small UFS2 partitions. Currently, installworld may fail if /usr has less than 140M-200M free (depending on tunefs(8) space/speed optimization setting and usage of soft-updates) that makes it difficult to upgrade legacy servers installed when 1GB dedicated partition was more than enough for /usr when /usr/{local|obj|src} are kept separately .

The patch does not change behaviour of install(1) if "-s" flag is not used. My efficiency tests were performed after some standard src.conf optimizations already applied and found not enough for legacy servers:

WITHOUT_KERNEL_SYMBOLS=
WITHOUT_DEBUG_FILES=
WITHOUT_TESTS=
WITHOUT_LLVM_TARGET_ALL=
WITH_LLVM_TARGET_X86=
WITHOUT_CLANG_FULL=

32% gained in addition to these options.

Test Plan

Use GEOM_NOP to collect write statistics.
First, create test partition (/dev/ada0s1f for example). 1GB is enough to run the test.
Then, assuming "make buildworld" already done and obj is populated:

kldload geom_nop
dev=/dev/ada0s1f
gnop create $dev
mkdir /data

r=260000
newfs -r $r -m 0 -o space $dev.nop
mount $dev.nop /data
mkdir /data/tmp /data/world
rm -rf /tmp && ln -s /data/tmp /tmp
cd /usr/src
make DESTDIR=/data/world installworld >/dev/null 2>&1
sync; sync; sync; sleep 3
gnop reset $dev.nop
gnop list > ~/gnop.prev
make DESTDIR=/data/world installworld > ~/installworld.log 2>&1
sync; sync; sync; sleep 3
gnop list > ~/gnop.new

Compare numbers in files gnop.prev (should be zeroes) and gnop.new to calculate amount of data sent to DESTDIR+/tmp during installworld. Run the test with unpatched and patched install(1) to see the difference.

Also, this mtree(8) command ensures that contents of DESTDIR is identical for unpatched and patched install(1) with exception of usr/bin/install binary itself if you use same obj contents for both passes:

mtree -c -k flags,gid,link,mode,nlink,size,type,uid,cksum -p /data/world > mtree.out

Note that install(1) belongs to bootstrap-tools so after patching its sources it is not enough to rebuild the utility but extra steps are required for new code to be used during installworld:

cd /usr/src/usr.bin/install && make clean all install
cd $(find -H /usr/obj -type d -path '*/tmp/*' -name xinstall) && rm xinstall xinstall.o
rm $(find -H /usr/obj -type f -path '*/legacy/*' -name install)
cd /usr/src && make _bootstrap-tools

[hrs]

I like the idea for optimization. Several comments are added in-line.

xinstall.c

863 ↗

(On Diff #74025)

I think strip() should not be called twice in this function.

Stripping should be done by the first call. Even if it fails, there is no reason to retry with no "-o", IMO. Do you have an example when the first call fails and the second call succeeds?

While I guess this can be a workaround for strip command with no "-o" support (as described below, "-o" is non-standard), I think install(1) should also fail in that case because people do not notice strip(1) is called twice every time install(1) is called when specifying strip(1) with no "-o" flag to STRIPBIN as well as "-s" flag is enabled. It will make slower than the original version.

1327 ↗

(On Diff #74025)

strip(1) is a POSIX command and "-o" is a non-standard extension. So I would suggest to enable "-o" optimization only when STRIPBIN is not defined implying use of the stock strip(1).

1335 ↗

(On Diff #74025)

Adding "--" might be better here. When from_name starts with "-", this command fails.

[eugen_grosbein.net]

Use: strip -o to_name -- from_name

[eugen_grosbein.net]

Thanks, I've fixed this.

[eugen_grosbein.net]

My goal is to improve performance by default not requiring any user actions while not breaking things same time. Both GNU binutils version of strip(1) and our version from contrib/elftoolchain/elfcopy support -o, so there would be no slowdown but improvement for all supported branches. However, install(1) could be used in some strange environment for cases other than "installworld" and I don't want it to break there making regression.

Enabling optimization depending on STRIPBIN for common case would defeat the whole purpose. If you like, I can disable usage of -o when STRIPBIN is defined cutting second "fallback" call.

[eugen_grosbein.net]

I'm going to commit this soon unless an objection is raised.