Bug#793404: massive waste of CPU time in debian/rules by inline commands
On 2015-07-25 10:27, Niels Thykier wrote:
> [...]
>
>> So your ":=" variable will be evaluated 4 times, or 7+N times if you use
>> dh.
>>
>> [...]
>>
>
> It should be doable to reduce the dh side of it to 1+N by caching the
> result of make (in a file-based cache).
>
> ~Niels
>
>
Correction, 2+N (of the dh caused one). Attached is a PoC patch for
implementing said cache. Unfortunately, it is only effective for
removing one parsing of debian/rules (2 when using -tc or doing repeated
builds from the same directory).
At this point, I am not entirely convinced the complexity is worth the
gains - especially not with Guillem's plans for dpkg/1.18.2, which is
likely to give more and make this patch less useful. I might reconsider
it if there is still an issue after next dpkg+debhelper upload.
Thanks,
~Niels
From 61c48ce2c5038a2cbd38b0cb63433e0589515c0e Mon Sep 17 00:00:00 2001
From: Niels Thykier <niels@thykier.net>
Date: Tue, 28 Jul 2015 22:41:41 +0200
Subject: [PATCH] dh: Cache target information from make
Cache which targets are empty and which are not. This avoids at least
1 runs over debian/rules in a regular build. The cache has to be
purged during clean, so the solution is rather limited.
Example of when the cache is used and when it is not:
dh clean <-- reads debian/rules
...
dh build <-- reads debian/rules (clean leaves no cache)
...
dh binary <-- uses cache
...
dh clean <-- uses cache but purges it afterwards
The latter "dh clean" happens with repeated builds or
dpkg-buildpackage -tc.
Signed-off-by: Niels Thykier <niels@thykier.net>
---
debian/changelog | 4 ++++
dh | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 61 insertions(+)
diff --git a/debian/changelog b/debian/changelog
index 650b294..62f1a8b 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -22,6 +22,10 @@ debhelper (9.20150629) UNRELEASED; urgency=medium
(Closes: #793443)
* dh_strip: Always compress debug sections of debug symbols
in ddebs.
+ * dh: Cache the output of make(1) to reduce the number of times
+ debian/rules is evaluated. This will primarily speed up
+ builds that use many or expensive ":=" variables in their
+ debian/rules files (Fore more info, please see #793404)
-- Niels Thykier <niels@thykier.net> Sun, 28 Jun 2015 15:08:19 +0200
diff --git a/dh b/dh
index 07facd0..e5050ed 100755
--- a/dh
+++ b/dh
@@ -343,6 +343,9 @@ if ($sequence eq 'debian/rules' ||
exit 0;
}
+# Some cache variables
+my $CACHE_DIR = 'debian/.debhelper';
+my $RULES_CACHE = "${CACHE_DIR}/rules-target";
# Definitions of sequences.
my %sequences;
@@ -647,6 +650,14 @@ elsif ($dh{BEFORE}) {
$stoppoint=command_pos($dh{BEFORE}, @sequence) - 1;
}
+if (not $dh{NO_ACT} and @sequence and $sequence[-1] ne 'dh_clean') {
+ # Only write the cache if there is something in the sequence (in
+ # the off chance that someone had a --remaining after dh_clean).
+ # Also, don't bother writing the cache if the sequence will remove
+ # it (clean does not imply any dh <target> calls).
+ update_rules_cache();
+}
+
# Now run the commands in the sequence.
foreach my $i (0..$stoppoint) {
my $command=$sequence[$i];
@@ -843,6 +854,26 @@ sub rules {
{
my %targets;
my $rules_parsed;
+my $cache_outdated;
+
+sub update_rules_cache {
+ return if not $cache_outdated;
+
+ install_dir($CACHE_DIR);
+ # Ignore errors - it is not a critical file
+ if (open(my $fd, '>', $RULES_CACHE)) {
+ my (@explicit, @empty);
+ for my $target (keys(%targets)) {
+ if ($targets{$target}) {
+ push(@explicit, $target);
+ } else {
+ push(@empty, $target);
+ }
+ }
+ print {$fd} 'explicit: ' . join(',', @explicit) . "\n" if @explicit;
+ print {$fd} 'empty: ' . join(',', @empty) . "\n" if @empty;
+ }
+}
sub rules_explicit_target {
# Checks if a specified target exists as an explicit target
@@ -850,6 +881,32 @@ sub rules_explicit_target {
# undef is returned if target does not exist, 0 if target is noop
# and 1 if target has dependencies or executes commands.
my $target=shift;
+ if (not $rules_parsed) {
+ # Check if we have a (valid) cache from the last run.
+ my $mtime_rules = ((stat('debian/rules'))[9]) // 0;
+ my $mtime_cache = ((stat($RULES_CACHE))[9]) // -1;
+ # Ignore errors from open - it is not a critical file.
+ if ($mtime_cache > $mtime_rules and open(my $fd, '<', $RULES_CACHE)) {
+ while (my $line = <$fd>) {
+ my $value;
+ chomp($line);
+ if ($line =~ s/^explicit: //) {
+ $value = 1;
+ } elsif ($line =~ s/^empty: //) {
+ $value = 0;
+ } else {
+ next;
+ }
+ for my $target (split(' ', $line)) {
+ $targets{$target} = $value;
+ }
+ }
+ $rules_parsed = 1;
+ close($fd);
+ } else {
+ $cache_outdated = 1;
+ }
+ }
if (! $rules_parsed) {
my $processing_targets = 0;
--
2.4.6
Reply to: