Hi!
On Mon, 2025-01-06 at 21:22:20 +0100, Helge Deller wrote:
> On 1/6/25 20:59, Stephen Kitt wrote:
> > On Sun, Jan 05, 2025 at 11:17:33PM +0100, Helge Deller wrote:
> > > I was able to narrow the issue down.
> > > make-dfsg (4.4.1-1) is introducing the issue.
> > >
> > > Reverting "make" back down to version 4.3-4.1 solves the issue
> > > and lets "dpkg-buildpackage" run lightning fast again.
> > >
> > > Looking at the make changelog, this seems related:
> > > * New upstream version 4.4. Closes: #1029106.
> > > - Exports variables to commands started by $(shell ...).
> >
> > That is indeed probably the reason; I’ve seen more significant speed
> > degradation in gcc-mingw-w64 for example (fixed by avoiding recursive
> > expansions).
> >
> > Could you qualify “incredible slow”? In my tests, vim fails its tests
> > so I can’t measure the speed difference, and neomutt went from a 1 min
> > build in testing (on my 10-year-old system) to 1:30, which is
> > significant but not incredibly slow in my book. There is a noticeable
> > pause whenever debian/rules is loaded, so Make is definitely doing too
> > much work.
>
> True.
>
> You probably won't notice on x86 and other fast machines.
> Sometimes there the build takes 2 minutes instead of prior 1 minute.
> So, it's 100% slower, but not a real problem.
>
> That's different on parisc:
> Physical parisc machines need to flush caches when a processes is spawned.
> Cache flushes are generally slow and I think this is a problem on other
> architectures (e.g. sparc?) as well.
> It's even worse is on those buildds which are running linux-user.
> Everytime a process is spawned, linux-user needs to assembly the file again,
> and the build gets slowed down a lot.
> Example:
> https://buildd.debian.org/status/logs.php?pkg=neomutt&arch=hppa
> On the physical machine "parisc" build time went up from 20 minutes
> (2024-11-24)
> to 1:10 hours (2025-01-05).
>
> > I’ll try to come up with a minimal reproducer and forward the issue
> > upstream.
>
> Btw, I got some numbers how often dpkg_buildflags is called by running
> something like:
> strace -f dpkg-buildpackage -B -nc 2>&1 | grep buildflags > some_file
> wc -l some_file
I just got approached by Michael Hudson-Doyle (CCed) where something
similar was noticed in Ubuntu [U]. We pondered whether the dpkg-buildflags
had gotten very slow or the amount of calls had increased, Michael
tested calling «debian/rules clean» for libvmime 0.9.2-8.2ubuntu1,
with make 4.3 and 4.4, and noticed the following numbers on Ubuntu:
4.3 calls dpkg-buildflags 22 times
4.4 calls dpkg-buildflags 1492 times
For me the numbers on Debian were 23 and 138 respectively.
After testing whether removing the DPKG_BUILDFLAGS_EXPORT_ENVVAR support
might help (which it did not), I noticed that the buildflags.mk is using
the «or» make function for its variable caching, which can be problematic
when the variables expand to an empty string. Changing that to check
whether the cached variable was already defined (even if empty), takes
my numbers from 138 to 40, which is still not ideal but much better.
I've queued that patch for the next dpkg upload (attached).
Then I noticed that the dpkg_buildflags_setvar variable uses «=»
instead of «?=» for its internal variable assignment. Changing that
takes it down from 40 to 20, but the problem is that this change could
be problematic, as ISTM, it changes semantics, where «=» will
overwrite a previously assigned variable, so this could break
packages. :/ And I don't think this is a safe change to make. I'm
attaching the change anyway for people to take a peek, but I'm not
currently planning on queueing it. Maybe after a mass rebuild and
mass autopkgtest run or similar, if stuff does not break then that
could be considered.
Michael also noticed tons of messages from «make --debug=all», like:
/usr/share/dpkg//buildflags.mk:71: not recursively expanding CFLAGS to export
to shell function
It's still not clear what is going on with make 4.4, and what has
caused these regressions. The multiple braking behavior changes might
explain it, but the reason does not seem obvious. The buildflags.mk
change to check for the cached variable being defined, seems correct
anyway, and does not regress with make 4.3, so that looks like a good
improvement non the less, but w/o the ?= change we are not back to
the old numbers.
[U] https://bugs.launchpad.net/ubuntu/+source/dpkg/+bug/2093107
Thanks,
Guillem
From a17a914248fe5a128ca1bfd4d96f146c1dc40153 Mon Sep 17 00:00:00 2001
From: Guillem Jover <guil...@debian.org>
Date: Mon, 13 Jan 2025 02:07:48 +0100
Subject: [PATCH 1/2] scripts/mk: Use flavor to avoid re-caching empty
variables
We were using the or function for the variable caches, but if the
variable is empty we might end up re-evaluating the contents, which
means we can end up executing the same command many times.
Ref: #1092051
---
scripts/mk/architecture.mk | 2 +-
scripts/mk/buildflags.mk | 2 +-
scripts/mk/pkg-info.mk | 2 +-
scripts/mk/vendor.mk | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/scripts/mk/architecture.mk b/scripts/mk/architecture.mk
index e506fb672..ba081e07b 100644
--- a/scripts/mk/architecture.mk
+++ b/scripts/mk/architecture.mk
@@ -10,7 +10,7 @@
ifndef dpkg_architecture_mk_included
dpkg_architecture_mk_included = yes
-dpkg_lazy_eval ?= $$(or $$(value DPKG_CACHE_$(1)),$$(eval DPKG_CACHE_$(1) := $$(shell $(2)))$$(value DPKG_CACHE_$(1)))
+dpkg_lazy_eval ?= $$(if $$(filter undefined,$$(flavor DPKG_CACHE_$(1))),$$(eval DPKG_CACHE_$(1) := $$(shell $(2)))$$(value DPKG_CACHE_$(1)),$$(value DPKG_CACHE_$(1)))
dpkg_architecture_setvar = export $(1) ?= $(call dpkg_lazy_eval,$(1),dpkg-architecture -q$(1))
diff --git a/scripts/mk/buildflags.mk b/scripts/mk/buildflags.mk
index a273b7089..7998d99f5 100644
--- a/scripts/mk/buildflags.mk
+++ b/scripts/mk/buildflags.mk
@@ -38,7 +38,7 @@ dpkg_buildflags_mk_included = yes
# This list is kept in sync with the default set of flags returned
# by dpkg-buildflags.
-dpkg_lazy_eval ?= $$(or $$(value DPKG_CACHE_$(1)),$$(eval DPKG_CACHE_$(1) := $$(shell $(2)))$$(value DPKG_CACHE_$(1)))
+dpkg_lazy_eval ?= $$(if $$(filter undefined,$$(flavor DPKG_CACHE_$(1))),$$(eval DPKG_CACHE_$(1) := $$(shell $(2)))$$(value DPKG_CACHE_$(1)),$$(value DPKG_CACHE_$(1)))
DPKG_BUILDFLAGS_LIST := $(foreach var,\
ASFLAGS \
diff --git a/scripts/mk/pkg-info.mk b/scripts/mk/pkg-info.mk
index 017f51918..1cfd62121 100644
--- a/scripts/mk/pkg-info.mk
+++ b/scripts/mk/pkg-info.mk
@@ -48,7 +48,7 @@
ifndef dpkg_pkg_info_mk_included
dpkg_pkg_info_mk_included = yes
-dpkg_late_eval ?= $(or $(value DPKG_CACHE_$(1)),$(eval DPKG_CACHE_$(1) := $(shell $(2)))$(value DPKG_CACHE_$(1)))
+dpkg_late_eval ?= $(if $(filter undefined,$(flavor DPKG_CACHE_$(1))),$(eval DPKG_CACHE_$(1) := $(shell $(2)))$(value DPKG_CACHE_$(1)),$(value DPKG_CACHE_$(1)))
DEB_SOURCE = $(call dpkg_late_eval,DEB_SOURCE,dpkg-parsechangelog -SSource)
DEB_VERSION = $(call dpkg_late_eval,DEB_VERSION,dpkg-parsechangelog -SVersion)
diff --git a/scripts/mk/vendor.mk b/scripts/mk/vendor.mk
index 1d33299a0..18da22a56 100644
--- a/scripts/mk/vendor.mk
+++ b/scripts/mk/vendor.mk
@@ -45,7 +45,7 @@ ifndef dpkg_datadir
endif
include $(dpkg_datadir)/buildapi.mk
-dpkg_late_eval ?= $(or $(value DPKG_CACHE_$(1)),$(eval DPKG_CACHE_$(1) := $(shell $(2)))$(value DPKG_CACHE_$(1)))
+dpkg_late_eval ?= $(if $(filter undefined,$(flavor DPKG_CACHE_$(1))),$(eval DPKG_CACHE_$(1) := $(shell $(2)))$(value DPKG_CACHE_$(1)),$(value DPKG_CACHE_$(1)))
DEB_VENDOR = $(call dpkg_late_eval,DEB_VENDOR,dpkg-vendor --query Vendor)
DEB_PARENT_VENDOR = $(call dpkg_late_eval,DEB_PARENT_VENDOR,dpkg-vendor --query Parent)
--
2.47.1
From dff234e80eeee16cdd945071459da615a0791192 Mon Sep 17 00:00:00 2001
From: Guillem Jover <guil...@debian.org>
Date: Mon, 13 Jan 2025 02:39:56 +0100
Subject: [PATCH 2/2] scripts/mk: Use ?= instead of = in dpkg_buildflags_setvar
This fixes excess (doubled) dpkg-buildflags invocations with make 4.4,
but at the price of changing the semantics, as we no longer overwrite
the values for these variables, which can break existing packages.
FIXME: This change cannot be applied as is right now, due to the above
mentioned compatibility concerns.
---
scripts/mk/buildflags.mk | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/mk/buildflags.mk b/scripts/mk/buildflags.mk
index 7998d99f5..075f88b70 100644
--- a/scripts/mk/buildflags.mk
+++ b/scripts/mk/buildflags.mk
@@ -66,7 +66,7 @@ $(foreach flag,$(DPKG_BUILDFLAGS_LIST),\
$(foreach operation,SET STRIP APPEND PREPEND,\
$(eval $(call dpkg_buildflags_export_envvar,DEB_$(flag)_MAINT_$(operation)))))
-dpkg_buildflags_setvar = $(1) = $(call dpkg_lazy_eval,$(1),$(DPKG_BUILDFLAGS_EXPORT_ENVVAR) dpkg-buildflags --get $(1))
+dpkg_buildflags_setvar = $(1) ?= $(call dpkg_lazy_eval,$(1),$(DPKG_BUILDFLAGS_EXPORT_ENVVAR) dpkg-buildflags --get $(1))
$(foreach flag,$(DPKG_BUILDFLAGS_LIST),\
$(eval $(call dpkg_buildflags_setvar,$(flag))))
--
2.47.1
