Bug#589427: dh_builddeb: support for parallel builds of debs
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
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
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
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
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
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
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