Bug#589427: dh_builddeb: support for parallel builds of debs

2011-07-17 Thread Gergely Nagy
Gergely Nagy  writes:

> I'll add fork() error handling and the above change, retest & send an
> updated patch.

Updated patch attached below. As you suggested, $processes and
$max_procs are initialised to 1 now, and the process waiting was
reworked.

Waiting in the else branch had another bug: if we didn't fork, a package
could be skipped (in every case where $max_procs > 2, one package would
be skipped in the else branch).

With the updated patch, if $max_procs > 1, we wait until there's a slot
ready, and fork then. This way no package will get lost, and it's a
better solution anyway.

Seems to work fine now, with parallel=1, 2 and 4 aswell.

The else branch also handles errors from fork(), but the error message
could use some improvement (I'm terrible when it comes to error
messages).

-- 
|8]

>From 3a2acbce4e606ac60f4eb116ecb981a31257d0a8 Mon Sep 17 00:00:00 2001
From: Gergely Nagy 
Date: Sun, 17 Jul 2011 20:48:05 +0200
Subject: [PATCH] dh_builddeb: support for parallel builds of debs

Implement support for parallel deb builds, when DEB_BUILD_OPTIONS has
parallels=N - limiting the number of forked processes to N.

Requested-By: Kari Pahula 
Signed-Off-By: Gergely Nagy 
---
 dh_builddeb |   59 ++-
 1 files changed, 42 insertions(+), 17 deletions(-)

diff --git a/dh_builddeb b/dh_builddeb
index b15c943..26e1274 100755
--- a/dh_builddeb
+++ b/dh_builddeb
@@ -63,30 +63,55 @@ else {
 	$dh{FILENAME}="/$dh{FILENAME}";
 }
 
+my $processes=1;
+my $max_procs=1;
+if (defined $ENV{DEB_BUILD_OPTIONS} && $ENV{DEB_BUILD_OPTIONS}=~/parallel=(\d+)/) {
+	$max_procs=$1;
+}
+
 foreach my $package (@{$dh{DOPACKAGES}}) {
-	my $tmp=tmpdir($package);
-	if (exists $ENV{DH_ALWAYS_EXCLUDE} && length $ENV{DH_ALWAYS_EXCLUDE}) {
-		if (! compat(5)) {
-			complex_doit("find $tmp $dh{EXCLUDE_FIND} | xargs rm -rf");
+	my $pid=0;
+
+	if ($max_procs > 1) {
+		while ($processes > $max_procs) {
+			wait;
+			$processes--;
+		}
+		$pid=fork();
+	}
+
+	if ($pid == 0) {
+		my $tmp=tmpdir($package);
+		if (exists $ENV{DH_ALWAYS_EXCLUDE} && length $ENV{DH_ALWAYS_EXCLUDE}) {
+			if (! compat(5)) {
+complex_doit("find $tmp $dh{EXCLUDE_FIND} | xargs rm -rf");
+			}
+			else {
+# Old broken code here for compatibility. Does not
+# remove everything.
+complex_doit("find $tmp -name $_ | xargs rm -rf")
+	foreach split(":", $ENV{DH_ALWAYS_EXCLUDE});
+			}
+		}
+		if (! is_udeb($package)) {
+			doit("dpkg-deb", @{$dh{U_PARAMS}}, "--build", $tmp, $dh{DESTDIR}.$dh{FILENAME});
 		}
 		else {
-			# Old broken code here for compatibility. Does not
-			# remove everything.
-			complex_doit("find $tmp -name $_ | xargs rm -rf")
-foreach split(":", $ENV{DH_ALWAYS_EXCLUDE});
+			my $filename=$dh{FILENAME};
+			if (! $filename) {
+$filename="/".udeb_filename($package);
+			}
+			doit("dpkg-deb", @{$dh{U_PARAMS}}, "--build", $tmp, $dh{DESTDIR}.$filename);
 		}
-	}
-	if (! is_udeb($package)) {
-		doit("dpkg-deb", @{$dh{U_PARAMS}}, "--build", $tmp, $dh{DESTDIR}.$dh{FILENAME});
-	}
-	else {
-		my $filename=$dh{FILENAME};
-		if (! $filename) {
-			$filename="/".udeb_filename($package);
+		exit (0) if ($max_procs > 1);
+	} else {
+		if (!defined $pid) {
+			error("fork failed!");
 		}
-		doit("dpkg-deb", @{$dh{U_PARAMS}}, "--build", $tmp, $dh{DESTDIR}.$filename);
+		$processes++;
 	}
 }
+while (($max_procs > 0) && (wait != -1)) {}
 
 =head1 SEE ALSO
 
-- 
1.7.2.5



Bug#589427: dh_builddeb: support for parallel builds of debs

2011-07-17 Thread Gergely Nagy
Joey Hess  writes:

> Gergely Nagy wrote:
>> +my $processes=0;
>> +my $max_procs=0;
>
> Wouldn't it be simpler to default $max_procs to 1, rather than the
> current special-cased handling of 0 via $forked etc?

Good point, it would be.

>> +} else {
>
> Hmm, there is no check for fork having failed. ($pid would be undef)
>
>> +if (++$processes > 1) {
>> +wait;
>> +--$processes;
>
> It's impossible for this code block to not run, and it seems to cause
> only one child to run at a time, since $processes will always be
> incremented to 1, the child waited on, and then it decremented again.

Hrm, interesting. I did see 2 dpkg-deb processes run when testing with
ghc..

Right. $processes start with 0, and when the first forks, the if
evaluates to if (1 > 1) which will be false. It will wait on the next
one after that, though, so the parallellism caps at 2, which happened to
be what I was testing with. Ouch.

Off the top of my head, 'if (++$processes >= $max_procs)' should work:
when the number of processes reach $max_procs, it'll wait 'till there's
a slot free.

I'll add fork() error handling and the above change, retest & send an
updated patch.

-- 
|8]



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org



Bug#589427: dh_builddeb: support for parallel builds of debs

2011-07-17 Thread Joey Hess
Gergely Nagy wrote:
> +my $processes=0;
> +my $max_procs=0;

Wouldn't it be simpler to default $max_procs to 1, rather than the
current special-cased handling of 0 via $forked etc?

> + } else {

Hmm, there is no check for fork having failed. ($pid would be undef)

> + if (++$processes > 1) {
> + wait;
> + --$processes;

It's impossible for this code block to not run, and it seems to cause
only one child to run at a time, since $processes will always be
incremented to 1, the child waited on, and then it decremented again.

-- 
see shy jo


signature.asc
Description: Digital signature


Bug#589427: dh_builddeb: support for parallel builds of debs

2011-07-17 Thread Gergely Nagy
Gergely Nagy  writes:

> Joey Hess  writes:
>
>> I think this should only be done if DEB_BUILD_OPTIONS has parallel=N,
>> and should limit dpkg-deb processes to the N. If someone sends a patch
>> doing that, I'll apply it.
>
> Such a patch (against git) is attached below. I did little testing only:
> it doesn't break building without DEB_BUILD_OPTIONS=parallel=N, and it
> seems to work with parallel=4 too.

Just tested with ghc on my dual-core laptop:

$ time -v dh_builddeb
dpkg-deb: building package `ghc' in `../ghc_7.0.3-2_amd64.deb'.
dpkg-deb: building package `ghc-prof' in `../ghc-prof_7.0.3-2_amd64.deb'.
dpkg-deb: building package `ghc-doc' in `../ghc-doc_7.0.3-2_all.deb'.
dpkg-deb: building package `ghc-dynamic' in `../ghc-dynamic_7.0.3-2_amd64.deb'.
dpkg-deb: building package `ghc-haddock' in `../ghc-haddock_7.0.3-2_amd64.deb'.
dpkg-deb: building package `ghc6' in `../ghc6_7.0.3-2_all.deb'.
dpkg-deb: building package `ghc6-doc' in `../ghc6-doc_7.0.3-2_all.deb'.
dpkg-deb: building package `ghc6-prof' in `../ghc6-prof_7.0.3-2_all.deb'.

Command being timed: "dh_builddeb"
User time (seconds): 266.86
System time (seconds): 8.34
Percent of CPU this job got: 102%
Elapsed (wall clock) time (h:mm:ss or m:ss): 4:29.57

$ DEB_BUILD_OPTIONS=parallel=2 time -v dh_builddeb
dpkg-deb: building package `ghc-prof' in `../ghc-prof_7.0.3-2_amd64.deb'.
dpkg-deb: building package `ghc' in `../ghc_7.0.3-2_amd64.deb'.
dpkg-deb: building package `ghc-doc' in `../ghc-doc_7.0.3-2_all.deb'.
dpkg-deb: building package `ghc-dynamic' in `../ghc-dynamic_7.0.3-2_amd64.deb'.
dpkg-deb: building package `ghc-haddock' in `../ghc-haddock_7.0.3-2_amd64.deb'.
dpkg-deb: building package `ghc6' in `../ghc6_7.0.3-2_all.deb'.
dpkg-deb: building package `ghc6-doc' in `../ghc6-doc_7.0.3-2_all.deb'.
dpkg-deb: building package `ghc6-prof' in `../ghc6-prof_7.0.3-2_all.deb'.

Command being timed: "dh_builddeb"
User time (seconds): 265.59
System time (seconds): 1.83
Percent of CPU this job got: 193%
Elapsed (wall clock) time (h:mm:ss or m:ss): 2:18.20

-- 
|8]



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org



Bug#589427: dh_builddeb: support for parallel builds of debs

2011-07-17 Thread Gergely Nagy
Joey Hess  writes:

> I think this should only be done if DEB_BUILD_OPTIONS has parallel=N,
> and should limit dpkg-deb processes to the N. If someone sends a patch
> doing that, I'll apply it.

Such a patch (against git) is attached below. I did little testing only:
it doesn't break building without DEB_BUILD_OPTIONS=parallel=N, and it
seems to work with parallel=4 too.

-- 
|8]

>From efc0aa75c47328018e41a4629fa02fb9cff4dbd0 Mon Sep 17 00:00:00 2001
From: Gergely Nagy 
Date: Sun, 17 Jul 2011 12:20:41 +0200
Subject: [PATCH] dh_builddeb: support for parallel builds of debs

Implement support for parallel deb builds, when DEB_BUILD_OPTIONS has
parallels=N - limiting the number of forked processes to N.

Requested-By: Kari Pahula 
Signed-Off-By: Gergely Nagy 
---
 dh_builddeb |   56 +++-
 1 files changed, 39 insertions(+), 17 deletions(-)

diff --git a/dh_builddeb b/dh_builddeb
index b15c943..5378be5 100755
--- a/dh_builddeb
+++ b/dh_builddeb
@@ -63,30 +63,52 @@ else {
 	$dh{FILENAME}="/$dh{FILENAME}";
 }
 
+my $processes=0;
+my $max_procs=0;
+if (defined $ENV{DEB_BUILD_OPTIONS} && $ENV{DEB_BUILD_OPTIONS}=~/parallel=(\d+)/) {
+	$max_procs=$1;
+}
+
 foreach my $package (@{$dh{DOPACKAGES}}) {
-	my $tmp=tmpdir($package);
-	if (exists $ENV{DH_ALWAYS_EXCLUDE} && length $ENV{DH_ALWAYS_EXCLUDE}) {
-		if (! compat(5)) {
-			complex_doit("find $tmp $dh{EXCLUDE_FIND} | xargs rm -rf");
+	my ($pid, $forked) = (0, 0);
+
+	if ($processes < $max_procs) {
+		$pid=fork();
+		$forked=1;
+	}
+
+	if ($pid == 0) {
+		my $tmp=tmpdir($package);
+		if (exists $ENV{DH_ALWAYS_EXCLUDE} && length $ENV{DH_ALWAYS_EXCLUDE}) {
+			if (! compat(5)) {
+complex_doit("find $tmp $dh{EXCLUDE_FIND} | xargs rm -rf");
+			}
+			else {
+# Old broken code here for compatibility. Does not
+# remove everything.
+complex_doit("find $tmp -name $_ | xargs rm -rf")
+	foreach split(":", $ENV{DH_ALWAYS_EXCLUDE});
+			}
+		}
+		if (! is_udeb($package)) {
+			doit("dpkg-deb", @{$dh{U_PARAMS}}, "--build", $tmp, $dh{DESTDIR}.$dh{FILENAME});
 		}
 		else {
-			# Old broken code here for compatibility. Does not
-			# remove everything.
-			complex_doit("find $tmp -name $_ | xargs rm -rf")
-foreach split(":", $ENV{DH_ALWAYS_EXCLUDE});
+			my $filename=$dh{FILENAME};
+			if (! $filename) {
+$filename="/".udeb_filename($package);
+			}
+			doit("dpkg-deb", @{$dh{U_PARAMS}}, "--build", $tmp, $dh{DESTDIR}.$filename);
 		}
-	}
-	if (! is_udeb($package)) {
-		doit("dpkg-deb", @{$dh{U_PARAMS}}, "--build", $tmp, $dh{DESTDIR}.$dh{FILENAME});
-	}
-	else {
-		my $filename=$dh{FILENAME};
-		if (! $filename) {
-			$filename="/".udeb_filename($package);
+		exit (0) if ($forked);
+	} else {
+		if (++$processes > 1) {
+			wait;
+			--$processes;
 		}
-		doit("dpkg-deb", @{$dh{U_PARAMS}}, "--build", $tmp, $dh{DESTDIR}.$filename);
 	}
 }
+while (($max_procs > 0) && (wait != -1)) {}
 
 =head1 SEE ALSO
 
-- 
1.7.2.5



Bug#589427: dh_builddeb: support for parallel builds of debs

2011-07-16 Thread Joey Hess
Kari Pahula wrote:
> I did some tests on this, making a rather trivial change to
> dh_builddeb's foreach loop, like this:
> 
> my $processes = 0;
> foreach ... {
> my $pid = fork();
> if ($pid == 0) {
> ...
>   exit 0;
> } else {
>   if (++$processes > 1) {
>   wait;
>   --$processes;
>   }
> }
> }
> while (wait != -1) {}

I think this should only be done if DEB_BUILD_OPTIONS has parallel=N,
and should limit dpkg-deb processes to the N. If someone sends a patch
doing that, I'll apply it.

-- 
see shy jo


signature.asc
Description: Digital signature


Bug#589427: dh_builddeb: support for parallel builds of debs

2010-07-17 Thread Kari Pahula
Package: debhelper
Version: 7.9.3
Severity: wishlist

It'd be useful to have dh_builddeb have support for building debs in
parallel.  As a case, the ghc6 6.12.3 package I'm working on now has
debs sized 4M, 8M, 16M, 32M and 43M.  After building it all I'm fairly
sure to have it all in memory and the CPU is the bottleneck, running
gzip on all that data.  I'd love to pass some of that to the second
core.

I did some tests on this, making a rather trivial change to
dh_builddeb's foreach loop, like this:

my $processes = 0;
foreach ... {
my $pid = fork();
if ($pid == 0) {
...
exit 0;
} else {
if (++$processes > 1) {
wait;
--$processes;
}
}
}
while (wait != -1) {}

Here's time dh_builddeb with the current version:
real4m0.960s
user3m56.903s
sys 0m2.636s

And here's what I got with this (some noise in there but you get the
idea):
real2m21.527s
user3m54.475s
sys 0m2.536s



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org