| Message ID | 0f48abf2ac872957d6a4a150ead39564053f2afc.1589716209.git.mschiffer@universe-factory.net |
|---|---|
| State | Under Review |
| Delegated to: | Matthias Schiffer |
| Headers | show |
| Series | Switch to zstd for kernel debuginfo compression | expand |
On 5/17/20 1:51 PM, Matthias Schiffer wrote: > Comression level -19 was chosen as it provides a very good tradeoff > between compression ratio and performance, especially in multithreaded > operation. > > Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net> Jow, do you have any opinion on this? I assume this will also require changes to the phase2 builtbot config - at least the SDK download pattern, and installation of zstd in the build environment. Anything else? > --- > target/imagebuilder/Makefile | 8 ++++---- > target/sdk/Makefile | 8 ++++---- > 2 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/target/imagebuilder/Makefile b/target/imagebuilder/Makefile > index b463feb456ee..5c09109150a8 100644 > --- a/target/imagebuilder/Makefile > +++ b/target/imagebuilder/Makefile > @@ -21,7 +21,7 @@ IB_IDIR:=$(patsubst $(TOPDIR)/%,$(PKG_BUILD_DIR)/%,$(STAGING_DIR_IMAGE)) > > all: compile > > -$(BIN_DIR)/$(IB_NAME).tar.xz: clean > +$(BIN_DIR)/$(IB_NAME).tar.zst: clean > rm -rf $(PKG_BUILD_DIR) > mkdir -p $(IB_KDIR) $(IB_LDIR) $(PKG_BUILD_DIR)/staging_dir/host/lib \ > $(PKG_BUILD_DIR)/target $(PKG_BUILD_DIR)/scripts $(IB_DTSDIR) > @@ -86,12 +86,12 @@ endif > (cd $(PKG_BUILD_DIR); find staging_dir/host/bin/ $(IB_LDIR)/scripts/dtc/ -type f | \ > $(XARGS) $(SCRIPT_DIR)/bundle-libraries.sh $(PKG_BUILD_DIR)/staging_dir/host) > STRIP=sstrip $(SCRIPT_DIR)/rstrip.sh $(PKG_BUILD_DIR)/staging_dir/host/bin/ > - $(TAR) -cf - -C $(BUILD_DIR) $(IB_NAME) | xz -T$(if $(filter 1,$(NPROC)),2,0) -zc -7e > $@ > + $(TAR) -cf - -C $(BUILD_DIR) $(IB_NAME) | zstd -T0 -19 -f -o $@ > > download: > prepare: > -compile: $(BIN_DIR)/$(IB_NAME).tar.xz > +compile: $(BIN_DIR)/$(IB_NAME).tar.zst > install: compile > > clean: FORCE > - rm -rf $(PKG_BUILD_DIR) $(BIN_DIR)/$(IB_NAME).tar.xz > + rm -rf $(PKG_BUILD_DIR) $(BIN_DIR)/$(IB_NAME).tar.zst > diff --git a/target/sdk/Makefile b/target/sdk/Makefile > index 6d818347204a..13389c849958 100644 > --- a/target/sdk/Makefile > +++ b/target/sdk/Makefile > @@ -81,7 +81,7 @@ KERNEL_FILES := $(patsubst $(TOPDIR)/%,%,$(wildcard $(addprefix $(LINUX_DIR)/,$( > > all: compile > > -$(BIN_DIR)/$(SDK_NAME).tar.xz: clean > +$(BIN_DIR)/$(SDK_NAME).tar.zst: clean > mkdir -p $(SDK_BUILD_DIR)/dl $(SDK_BUILD_DIR)/package > $(CP) -L $(INCLUDE_DIR) $(SCRIPT_DIR) $(SDK_BUILD_DIR)/ > $(TAR) -cf - -C $(TOPDIR) \ > @@ -156,13 +156,13 @@ $(BIN_DIR)/$(SDK_NAME).tar.xz: clean > find $(SDK_BUILD_DIR) -name CVS | $(XARGS) rm -rf > -make -C $(SDK_BUILD_DIR)/scripts/config clean > (cd $(BUILD_DIR); \ > - tar -I 'xz -7e' -cf $@ $(SDK_NAME); \ > + tar -I 'zstd -T0 -19' -cf $@ $(SDK_NAME); \ > ) > > download: > prepare: > -compile: $(BIN_DIR)/$(SDK_NAME).tar.xz > +compile: $(BIN_DIR)/$(SDK_NAME).tar.zst > install: compile > > clean: > - rm -rf $(SDK_BUILD_DIR) $(BIN_DIR)/$(SDK_NAME).tar.xz > + rm -rf $(SDK_BUILD_DIR) $(BIN_DIR)/$(SDK_NAME).tar.zst >
On 5/29/20 9:18 PM, Matthias Schiffer wrote: > On 5/17/20 1:51 PM, Matthias Schiffer wrote: >> Comression level -19 was chosen as it provides a very good tradeoff >> between compression ratio and performance, especially in multithreaded >> operation. >> >> Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net> > > Jow, do you have any opinion on this? I assume this will also require > changes to the phase2 builtbot config - at least the SDK download pattern, > and installation of zstd in the build environment. Anything else? > I have applied the first 3 patches now, as they seem uncontroversial. For patch 4, I'd like to have an ACK from someone familiar with the buildbot setup. Matthias > >> --- >> target/imagebuilder/Makefile | 8 ++++---- >> target/sdk/Makefile | 8 ++++---- >> 2 files changed, 8 insertions(+), 8 deletions(-) >> >> diff --git a/target/imagebuilder/Makefile b/target/imagebuilder/Makefile >> index b463feb456ee..5c09109150a8 100644 >> --- a/target/imagebuilder/Makefile >> +++ b/target/imagebuilder/Makefile >> @@ -21,7 +21,7 @@ IB_IDIR:=$(patsubst $(TOPDIR)/%,$(PKG_BUILD_DIR)/%,$(STAGING_DIR_IMAGE)) >> >> all: compile >> >> -$(BIN_DIR)/$(IB_NAME).tar.xz: clean >> +$(BIN_DIR)/$(IB_NAME).tar.zst: clean >> rm -rf $(PKG_BUILD_DIR) >> mkdir -p $(IB_KDIR) $(IB_LDIR) $(PKG_BUILD_DIR)/staging_dir/host/lib \ >> $(PKG_BUILD_DIR)/target $(PKG_BUILD_DIR)/scripts $(IB_DTSDIR) >> @@ -86,12 +86,12 @@ endif >> (cd $(PKG_BUILD_DIR); find staging_dir/host/bin/ $(IB_LDIR)/scripts/dtc/ -type f | \ >> $(XARGS) $(SCRIPT_DIR)/bundle-libraries.sh $(PKG_BUILD_DIR)/staging_dir/host) >> STRIP=sstrip $(SCRIPT_DIR)/rstrip.sh $(PKG_BUILD_DIR)/staging_dir/host/bin/ >> - $(TAR) -cf - -C $(BUILD_DIR) $(IB_NAME) | xz -T$(if $(filter 1,$(NPROC)),2,0) -zc -7e > $@ >> + $(TAR) -cf - -C $(BUILD_DIR) $(IB_NAME) | zstd -T0 -19 -f -o $@ >> >> download: >> prepare: >> -compile: $(BIN_DIR)/$(IB_NAME).tar.xz >> +compile: $(BIN_DIR)/$(IB_NAME).tar.zst >> install: compile >> >> clean: FORCE >> - rm -rf $(PKG_BUILD_DIR) $(BIN_DIR)/$(IB_NAME).tar.xz >> + rm -rf $(PKG_BUILD_DIR) $(BIN_DIR)/$(IB_NAME).tar.zst >> diff --git a/target/sdk/Makefile b/target/sdk/Makefile >> index 6d818347204a..13389c849958 100644 >> --- a/target/sdk/Makefile >> +++ b/target/sdk/Makefile >> @@ -81,7 +81,7 @@ KERNEL_FILES := $(patsubst $(TOPDIR)/%,%,$(wildcard $(addprefix $(LINUX_DIR)/,$( >> >> all: compile >> >> -$(BIN_DIR)/$(SDK_NAME).tar.xz: clean >> +$(BIN_DIR)/$(SDK_NAME).tar.zst: clean >> mkdir -p $(SDK_BUILD_DIR)/dl $(SDK_BUILD_DIR)/package >> $(CP) -L $(INCLUDE_DIR) $(SCRIPT_DIR) $(SDK_BUILD_DIR)/ >> $(TAR) -cf - -C $(TOPDIR) \ >> @@ -156,13 +156,13 @@ $(BIN_DIR)/$(SDK_NAME).tar.xz: clean >> find $(SDK_BUILD_DIR) -name CVS | $(XARGS) rm -rf >> -make -C $(SDK_BUILD_DIR)/scripts/config clean >> (cd $(BUILD_DIR); \ >> - tar -I 'xz -7e' -cf $@ $(SDK_NAME); \ >> + tar -I 'zstd -T0 -19' -cf $@ $(SDK_NAME); \ >> ) >> >> download: >> prepare: >> -compile: $(BIN_DIR)/$(SDK_NAME).tar.xz >> +compile: $(BIN_DIR)/$(SDK_NAME).tar.zst >> install: compile >> >> clean: >> - rm -rf $(SDK_BUILD_DIR) $(BIN_DIR)/$(SDK_NAME).tar.xz >> + rm -rf $(SDK_BUILD_DIR) $(BIN_DIR)/$(SDK_NAME).tar.zst >> > > > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel >
Matthias Schiffer <mschiffer@universe-factory.net> [2020-05-31 11:08:47]: Hi, > For patch 4, I'd like to have an ACK from someone familiar with the buildbot > setup. Disclaimer: I'm Buildbot setup noob, just helping occasionally with the maintenance burden, but from my limited understanding it's not just about ACK, some additional work is needed: 1. Someone has to provide patches against Buildbot[1] repo with the zstd naming changes 2. Someone has to build and publish updated Docker images for buildbot master/slave 3. Someone has to deploy this on machines under our control (having root access), we need to notify owners of the buildslave machines to upgrade their buildslaves to the updated Docker images built/published in step 2. I can probably help with 2. and 3., I would ideally done this with Buildbot version bump which is in the works[1], so we don't bother people with buildslaves upgrades that often. 1. https://git.openwrt.org/?p=buildbot.git 2. https://git.openwrt.org/?p=buildbot.git;a=shortlog;h=refs/heads/buildbot-2.4.1 -- ynezz
On 6/1/20 12:21 PM, Petr Štetiar wrote: > Matthias Schiffer <mschiffer@universe-factory.net> [2020-05-31 11:08:47]: > > Hi, > >> For patch 4, I'd like to have an ACK from someone familiar with the buildbot >> setup. > > Disclaimer: I'm Buildbot setup noob, just helping occasionally with the > maintenance burden, but from my limited understanding it's not just about ACK, > some additional work is needed: > > 1. Someone has to provide patches against Buildbot[1] repo with the zstd > naming changes I can write the patch, but it seems this is only a default value, and the config file defines the actual value - which makes sense, as only master snapshots would use the new value, while 19.07 and older would still use .xz. Where are these per-instances variables defined? Matthias > 2. Someone has to build and publish updated Docker images for buildbot master/slave > 3. Someone has to deploy this on machines under our control (having root > access), we need to notify owners of the buildslave machines to upgrade their > buildslaves to the updated Docker images built/published in step 2. > > I can probably help with 2. and 3., I would ideally done this with Buildbot > version bump which is in the works[1], so we don't bother people with > buildslaves upgrades that often. > > 1. https://git.openwrt.org/?p=buildbot.git > 2. https://git.openwrt.org/?p=buildbot.git;a=shortlog;h=refs/heads/buildbot-2.4.1 > > -- ynezz >
Matthias Schiffer <mschiffer@universe-factory.net> [2020-06-01 14:38:16]: Hi, > On 6/1/20 12:21 PM, Petr Štetiar wrote: > > > > 1. Someone has to provide patches against Buildbot[1] repo with the zstd > > naming changes > > I can write the patch, but it seems this is only a default value, and the > config file defines the actual value - which makes sense, as only master > snapshots would use the new value, while 19.07 and older would still use > .xz. Where are these per-instances variables defined? Ah, right, it's in roles/buildslave/templates/config.ini.j2 in ansible.git [rsync] {% if master_data.branch == "master" %} sdk_pattern = openwrt-sdk-*.tar.xz {% else %} sdk_pattern = {{ distribution }}-sdk-*.tar.xz {% endif %} I can change this Ansible part once needed. -- ynezz
Hi Petr and Matthias Le lun. 1 juin 2020 à 06:21, Petr Štetiar <ynezz@true.cz> a écrit : > Matthias Schiffer <mschiffer@universe-factory.net> [2020-05-31 11:08:47]: > > Hi, > > > For patch 4, I'd like to have an ACK from someone familiar with the > buildbot > > setup. > > Disclaimer: I'm Buildbot setup noob, just helping occasionally with the > maintenance burden, but from my limited understanding it's not just about > ACK, > some additional work is needed: > > 1. Someone has to provide patches against Buildbot[1] repo with the zstd > naming changes > 2. Someone has to build and publish updated Docker images for buildbot > master/slave > 3. Someone has to deploy this on machines under our control (having root > access), we need to notify owners of the buildslave machines to > upgrade their > buildslaves to the updated Docker images built/published in step 2. > 4. Fixup some of the feed repo CI https://github.com/openwrt/packages/blob/master/.circleci/config.yml#L9 https://github.com/openwrt/telephony/blob/master/.circleci/config.yml#L9 (not sure the docker image has zstd, might also need to rebuild it) Regards Etienne > I can probably help with 2. and 3., I would ideally done this with Buildbot > version bump which is in the works[1], so we don't bother people with > buildslaves upgrades that often. > > 1. https://git.openwrt.org/?p=buildbot.git > 2. > https://git.openwrt.org/?p=buildbot.git;a=shortlog;h=refs/heads/buildbot-2.4.1 > > -- ynezz > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel >
Hi, I tend to NAK this as it has the potential to interrupt a lot of downstream tooling. Up until now I also never heard about about Zstd - while this doesn't mean anything in the grand scheme of things I think it is a bit too "new" to be used widely yet. Seems tar only supports it since beginning of 2019. ~ Jo
diff --git a/target/imagebuilder/Makefile b/target/imagebuilder/Makefile index b463feb456ee..5c09109150a8 100644 --- a/target/imagebuilder/Makefile +++ b/target/imagebuilder/Makefile @@ -21,7 +21,7 @@ IB_IDIR:=$(patsubst $(TOPDIR)/%,$(PKG_BUILD_DIR)/%,$(STAGING_DIR_IMAGE)) all: compile -$(BIN_DIR)/$(IB_NAME).tar.xz: clean +$(BIN_DIR)/$(IB_NAME).tar.zst: clean rm -rf $(PKG_BUILD_DIR) mkdir -p $(IB_KDIR) $(IB_LDIR) $(PKG_BUILD_DIR)/staging_dir/host/lib \ $(PKG_BUILD_DIR)/target $(PKG_BUILD_DIR)/scripts $(IB_DTSDIR) @@ -86,12 +86,12 @@ endif (cd $(PKG_BUILD_DIR); find staging_dir/host/bin/ $(IB_LDIR)/scripts/dtc/ -type f | \ $(XARGS) $(SCRIPT_DIR)/bundle-libraries.sh $(PKG_BUILD_DIR)/staging_dir/host) STRIP=sstrip $(SCRIPT_DIR)/rstrip.sh $(PKG_BUILD_DIR)/staging_dir/host/bin/ - $(TAR) -cf - -C $(BUILD_DIR) $(IB_NAME) | xz -T$(if $(filter 1,$(NPROC)),2,0) -zc -7e > $@ + $(TAR) -cf - -C $(BUILD_DIR) $(IB_NAME) | zstd -T0 -19 -f -o $@ download: prepare: -compile: $(BIN_DIR)/$(IB_NAME).tar.xz +compile: $(BIN_DIR)/$(IB_NAME).tar.zst install: compile clean: FORCE - rm -rf $(PKG_BUILD_DIR) $(BIN_DIR)/$(IB_NAME).tar.xz + rm -rf $(PKG_BUILD_DIR) $(BIN_DIR)/$(IB_NAME).tar.zst diff --git a/target/sdk/Makefile b/target/sdk/Makefile index 6d818347204a..13389c849958 100644 --- a/target/sdk/Makefile +++ b/target/sdk/Makefile @@ -81,7 +81,7 @@ KERNEL_FILES := $(patsubst $(TOPDIR)/%,%,$(wildcard $(addprefix $(LINUX_DIR)/,$( all: compile -$(BIN_DIR)/$(SDK_NAME).tar.xz: clean +$(BIN_DIR)/$(SDK_NAME).tar.zst: clean mkdir -p $(SDK_BUILD_DIR)/dl $(SDK_BUILD_DIR)/package $(CP) -L $(INCLUDE_DIR) $(SCRIPT_DIR) $(SDK_BUILD_DIR)/ $(TAR) -cf - -C $(TOPDIR) \ @@ -156,13 +156,13 @@ $(BIN_DIR)/$(SDK_NAME).tar.xz: clean find $(SDK_BUILD_DIR) -name CVS | $(XARGS) rm -rf -make -C $(SDK_BUILD_DIR)/scripts/config clean (cd $(BUILD_DIR); \ - tar -I 'xz -7e' -cf $@ $(SDK_NAME); \ + tar -I 'zstd -T0 -19' -cf $@ $(SDK_NAME); \ ) download: prepare: -compile: $(BIN_DIR)/$(SDK_NAME).tar.xz +compile: $(BIN_DIR)/$(SDK_NAME).tar.zst install: compile clean: - rm -rf $(SDK_BUILD_DIR) $(BIN_DIR)/$(SDK_NAME).tar.xz + rm -rf $(SDK_BUILD_DIR) $(BIN_DIR)/$(SDK_NAME).tar.zst
Comression level -19 was chosen as it provides a very good tradeoff between compression ratio and performance, especially in multithreaded operation. Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net> --- target/imagebuilder/Makefile | 8 ++++---- target/sdk/Makefile | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-)