Re: [gentoo-portage-dev] [RFC/PATCH] prepstrip/ecompressdir: parallelize operations
On Monday 14 May 2012 18:42:07 Zac Medico wrote: > On 05/14/2012 01:10 PM, Mike Frysinger wrote: > > On Monday 14 May 2012 15:08:32 Zac Medico wrote: > >> Actually, the inode_var_name thing will not work unless it's all in > >> one process. > > > > hmm, true, but that's the level we currently parallelize at, so it's > > fine. we do one subprocess per ELF and that includes the > > strip/splitdebug/splitsources. > > > > parallelizing more than on a per-ELF basis will require much finer > > grained queues which, while possible, would make the file much harder to > > hack on and extend. and i'm not sure we'd see that much of a gain. > > The thing is, in the case of hardlinks, we're parallelizing multiple > times on the *same* elf. Anyway, I've fixed it by using a directory full > of hardlinks, in these commits: well, realistically speaking, hardlinking has been broken before the parallelization work. https://bugs.gentoo.org/400767 > http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=ad9442 > 75b88a50d2a1f694826b127cceb9221e78 > http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=9ed00 > a9e70a3705164a5349145ff467e5c40ddfd i'll go through it and see what's what -mike signature.asc Description: This is a digitally signed message part.
Re: [gentoo-portage-dev] [RFC/PATCH] prepstrip/ecompressdir: parallelize operations
On 05/14/2012 01:10 PM, Mike Frysinger wrote: > On Monday 14 May 2012 15:08:32 Zac Medico wrote: >> Actually, the inode_var_name thing will not work unless it's all in >> one process. > > hmm, true, but that's the level we currently parallelize at, so it's fine. > we > do one subprocess per ELF and that includes the strip/splitdebug/splitsources. > > parallelizing more than on a per-ELF basis will require much finer grained > queues which, while possible, would make the file much harder to hack on and > extend. and i'm not sure we'd see that much of a gain. > -mike The thing is, in the case of hardlinks, we're parallelizing multiple times on the *same* elf. Anyway, I've fixed it by using a directory full of hardlinks, in these commits: http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=ad944275b88a50d2a1f694826b127cceb9221e78 http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=9ed00a9e70a3705164a5349145ff467e5c40ddfd -- Thanks, Zac
Re: [gentoo-portage-dev] [RFC/PATCH] prepstrip/ecompressdir: parallelize operations
On Monday 14 May 2012 15:08:32 Zac Medico wrote: > Actually, the inode_var_name thing will not work unless it's all in > one process. hmm, true, but that's the level we currently parallelize at, so it's fine. we do one subprocess per ELF and that includes the strip/splitdebug/splitsources. parallelizing more than on a per-ELF basis will require much finer grained queues which, while possible, would make the file much harder to hack on and extend. and i'm not sure we'd see that much of a gain. -mike signature.asc Description: This is a digitally signed message part.
Re: [gentoo-portage-dev] [RFC/PATCH] prepstrip/ecompressdir: parallelize operations
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 05/14/2012 12:02 PM, Zac Medico wrote: > On 05/14/2012 11:53 AM, Mike Frysinger wrote: >> On Monday 14 May 2012 13:37:40 Mike Frysinger wrote: >>> On Monday 14 May 2012 04:44:12 Zac Medico wrote: http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=b4fb > a3 e9fa2e285244de491f57700978158c1838 >>> >>> should really fix it to make the code parallel safe rather >>> than disabling it completely. i'll work on that. > >> this should make it parallel safe -mike > > Yeah, that looks good. Actually, the inode_var_name thing will not work unless it's all in one process. - -- Thanks, Zac -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.19 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk+xWDAACgkQ/ejvha5XGaM8OwCguDf5rKVv4cpEmOYoqwrLBgGM mr0AniCfHtJiNJRpF+mC4oHquO3nSen1 =3gSf -END PGP SIGNATURE-
Re: [gentoo-portage-dev] [RFC/PATCH] prepstrip/ecompressdir: parallelize operations
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 05/14/2012 11:53 AM, Mike Frysinger wrote: > On Monday 14 May 2012 13:37:40 Mike Frysinger wrote: >> On Monday 14 May 2012 04:44:12 Zac Medico wrote: >>> http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=b4fb >>> >>> a3 e9fa2e285244de491f57700978158c1838 >> >> should really fix it to make the code parallel safe rather than >> disabling it completely. i'll work on that. > > this should make it parallel safe -mike Yeah, that looks good. - -- Thanks, Zac -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.19 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk+xVuEACgkQ/ejvha5XGaNa1ACeLTRHjwNuRRXp9wsLgKeTcKEp W7QAn2Z642Dx8r2OhDSifoqZtljFn7+E =piRb -END PGP SIGNATURE-
Re: [gentoo-portage-dev] [RFC/PATCH] prepstrip/ecompressdir: parallelize operations
On Monday 14 May 2012 13:37:40 Mike Frysinger wrote:
> On Monday 14 May 2012 04:44:12 Zac Medico wrote:
> > http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=b4fb
> > a3 e9fa2e285244de491f57700978158c1838
>
> should really fix it to make the code parallel safe rather than disabling
> it completely. i'll work on that.
this should make it parallel safe
-mike
--- a/bin/ebuild-helpers/prepstrip
+++ b/bin/ebuild-helpers/prepstrip
@@ -62,22 +62,13 @@
prepstrip_sources_dir=${EPREFIX}/usr/src/debug/${CATEGORY}/${PF}
type -P debugedit >/dev/null && debugedit_found=true || debugedit_found=false
debugedit_warned=false
-disable_parallel=false
-${FEATURES_splitdebug} && disable_parallel=true
-${FEATURES_installsources} && \
- ! ${RESTRICT_installsources} && \
- ${debugedit_found} && disable_parallel=true
-
-if ${disable_parallel} ; then
- # Disable parallel processing, in order to prevent interference with
- # temp files like debug.sources or prepstrip.split.debug
- numjobs() {
- echo 1
- }
-fi
-
multijob_init
+# Setup $T filesystem layout that we care about.
+tmpdir="${T}/prepstrip"
+rm -rf "${tmpdir}"
+mkdir -p "${tmpdir}"/{splitdebug,sources}
+
unset ${!INODE_*}
# Usage: inode_var_name:
@@ -112,11 +103,11 @@ save_elf_sources() {
buildid=$(debugedit -i \
-b "${WORKDIR}" \
-d "${prepstrip_sources_dir}" \
- -l "${T}"/debug.sources \
+ -l "${tmpdir}/sources/${x##*/}.${BASHPID}" \
"${x}")
}
-# Usage: save_elf_debug
+# Usage: save_elf_debug [splitdebug file]
save_elf_debug() {
${FEATURES_splitdebug} || return 0
@@ -125,6 +116,7 @@ save_elf_debug() {
# twice in this path) in order for gdb's debug-file-directory
# lookup to work correctly.
local x=$1
+ local splitdebug=$2
local y=${ED}usr/lib/debug/${x:${#D}}.debug
# dont save debug info twice
@@ -137,8 +129,8 @@ save_elf_debug() {
ln "${ED}usr/lib/debug/${!inode:${#D}}.debug" "${y}"
else
eval ${inode}=\${x}
- if [[ -e ${T}/prepstrip.split.debug ]] ; then
- mv "${T}"/prepstrip.split.debug "${y}"
+ if [[ -n ${splitdebug} ]] ; then
+ mv "${splitdebug}" "${y}"
else
local objcopy_flags="--only-keep-debug"
${FEATURES_compressdebug} && objcopy_flags+="
--compress-debug-sections"
@@ -175,11 +167,13 @@ process_elf() {
if ${strip_this} ; then
# see if we can split & strip at the same time
if [[ -n ${SPLIT_STRIP_FLAGS} ]] ; then
+ local shortname="${x##*/}.debug"
+ local
splitdebug="${tmpdir}/splitdebug/${shortname}.${BASHPID}"
${STRIP} ${strip_flags} \
- -f "${T}"/prepstrip.split.debug \
- -F "${x##*/}.debug" \
+ -f "${splitdebug}" \
+ -F "${shortname}" \
"${x}"
- save_elf_debug "${x}"
+ save_elf_debug "${x}" "${splitdebug}"
else
save_elf_debug "${x}"
${STRIP} ${strip_flags} "${x}"
@@ -194,8 +188,8 @@ if ! ${RESTRICT_binchecks} && ! ${RESTRICT_strip} ; then
# We need to do the non-stripped scan serially first before we turn
around
# and start stripping the files ourselves. The log parsing can be done
in
# parallel though.
- log=$T/scanelf-already-stripped.log
- scanelf -yqRBF '#k%F' -k '!.symtab' "$@" | sed -e "s#^${ED}##" > "$log"
+ log=${tmpdir}/scanelf-already-stripped.log
+ scanelf -yqRBF '#k%F' -k '!.symtab' "$@" | sed -e "s#^${ED}##" >
"${log}"
(
multijob_child_init
qa_var="QA_PRESTRIPPED_${ARCH/-/_}"
@@ -286,9 +280,11 @@ do
multijob_post_fork
done
-# Let jobs finish before processing ${T}/debug.sources
+# With a bit more work, we could run the rsync processes below in
+# parallel, but not sure that'd be an overall improvement.
multijob_finish
+cat "${tmpdir}"/sources/* > "${tmpdir}/debug.sources" 2>/dev/null
if [[ -s ${T}/debug.sources ]] && \
${FEATURES_installsources} && \
! ${RESTRICT_installsources} && \
signature.asc
Description: This is a digitally signed message part.
Re: [gentoo-portage-dev] [RFC/PATCH] prepstrip/ecompressdir: parallelize operations
On Monday 14 May 2012 04:44:12 Zac Medico wrote:
> On 05/14/2012 12:33 AM, Michael Haubenwallner wrote:
> >> +multijob_post_fork() {
> >> + : $(( ++mj_num_jobs ))
> >> + if [[ ${mj_num_jobs} -ge ${mj_max_jobs} ]] ; then
> >> + multijob_finish_one
> >
> > Feels like ignoring this child's exitstatus isn't intentional here.
>
> http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=2adc44
> 295a5b5c77640c32cd24ebbd8d52e5237b
simpler:
--- a/bin/helper-functions.sh
+++ b/bin/helper-functions.sh
@@ -54,11 +54,9 @@ multijob_finish() {
}
multijob_post_fork() {
- local ret=0
: $(( ++mj_num_jobs ))
if [[ ${mj_num_jobs} -ge ${mj_max_jobs} ]] ; then
multijob_finish_one
- : $(( ret |= $? ))
fi
- return ${ret}
+ return $?
}
> http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=b4fba3
> e9fa2e285244de491f57700978158c1838
should really fix it to make the code parallel safe rather than disabling it
completely. i'll work on that.
> http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=c534e
> 32f78cf7c543e9203e7fe1c7b1630144ffb
forking & waiting for a single child doesn't make much sense. might as well
not fork at all. but this can still be parallelizied a little:
--- a/bin/ebuild-helpers/prepstrip
+++ b/bin/ebuild-helpers/prepstrip
@@ -187,12 +187,15 @@ process_elf() {
# We want to log already stripped binaries, as this may be a QA violation.
# They prevent us from getting the splitdebug data.
if ! ${RESTRICT_binchecks} && ! ${RESTRICT_strip} ; then
+ # We need to do the non-stripped scan serially first before we turn
around
+ # and start stripping the files ourselves. The log parsing can be done
in
+ # parallel though.
+ log=$T/scanelf-already-stripped.log
+ scanelf -yqRBF '#k%F' -k '!.symtab' "$@" | sed -e "s#^${ED}##" > "$log"
(
multijob_child_init
- log=$T/scanelf-already-stripped.log
qa_var="QA_PRESTRIPPED_${ARCH/-/_}"
[[ -n ${!qa_var} ]] && QA_PRESTRIPPED="${!qa_var}"
- scanelf -yqRBF '#k%F' -k '!.symtab' "$@" | sed -e "s#^${ED}##" > "$log"
if [[ -n $QA_PRESTRIPPED && -s $log && \
${QA_STRICT_PRESTRIPPED-unset} = unset ]] ; then
shopts=$-
@@ -215,9 +218,6 @@ if ! ${RESTRICT_binchecks} && ! ${RESTRICT_strip} ; then
multijob_post_fork
fi
-# Let the Pre-stripped check finish before we start stripping
-multijob_finish
-
# Now we look for unstripped binaries.
for x in \
$(scanelf -yqRBF '#k%F' -k '.symtab' "$@") \
-mike
signature.asc
Description: This is a digitally signed message part.
Re: [gentoo-portage-dev] [RFC/PATCH] prepstrip/ecompressdir: parallelize operations
On Monday 14 May 2012 03:33:58 Michael Haubenwallner wrote:
> On 05/11/2012 06:39 PM, Mike Frysinger wrote:
> > +multijob_child_init() {
> > + trap 'echo ${BASHPID} $? >&'${mj_control_fd} EXIT
> > + trap 'exit 1' INT TERM
> > +}
>
> Just wondering why $! in parent isn't used anywhere, even not for some
> integrity check if the child's BASHPID actually was forked by parent.
i don't know of any cases where this would error out. if there are too many
processes, bash itself will retry a few times before aborting. so checking $!
wouldn't help.
keep in mind, what you're proposing is basically checking the return value of
fork(), and that can fail in very few ways. all of which, afaik, bash does
not bubble up to the script.
-mike
signature.asc
Description: This is a digitally signed message part.
Re: [gentoo-portage-dev] [RFC/PATCH] prepstrip/ecompressdir: parallelize operations
On Mon, May 14, 2012 at 09:33:58AM +0200, Michael Haubenwallner wrote:
>
>
> On 05/11/2012 06:39 PM, Mike Frysinger wrote:
> > +multijob_child_init() {
> > + trap 'echo ${BASHPID} $? >&'${mj_control_fd} EXIT
> > + trap 'exit 1' INT TERM
> > +}
>
> Just wondering why $! in parent isn't used anywhere, even not for some
> integrity check if the child's BASHPID actually was forked by parent.
wait'ing on it can fail; roughly bash basically reaps on it's own
(uncontrollably so), but bash still will actually do the wait,
basically falling back to it's internal list of what it reaped.
That's *roughly* what I got out of it when I wrote what vapier's
ape'ing here, and is exactly the issue that bit me in the ass on a 48
core. If things are moving fast enough, sooner or later that whacky
wait behaviour will intersect a real pid, one that isn't a direct
child, and bash will puke a horrible error.
That pretty much leaves you w/ 'wait jobspec' or 'wait' to clean up
the bash innards. This exact issue is why the code passes the exit
status back.
~harring
Re: [gentoo-portage-dev] [RFC/PATCH] prepstrip/ecompressdir: parallelize operations
On 05/14/2012 12:33 AM, Michael Haubenwallner wrote:
>> +multijob_post_fork() {
>> +: $(( ++mj_num_jobs ))
>> +if [[ ${mj_num_jobs} -ge ${mj_max_jobs} ]] ; then
>> +multijob_finish_one
>
> Feels like ignoring this child's exitstatus isn't intentional here.
Thanks, fixed now:
http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=2adc44295a5b5c77640c32cd24ebbd8d52e5237b
And here are a couple of more related fixes:
http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=b4fba3e9fa2e285244de491f57700978158c1838
http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=c534e32f78cf7c543e9203e7fe1c7b1630144ffb
--
Thanks,
Zac
Re: [gentoo-portage-dev] [RFC/PATCH] prepstrip/ecompressdir: parallelize operations
On 05/11/2012 06:39 PM, Mike Frysinger wrote:
> +multijob_child_init() {
> + trap 'echo ${BASHPID} $? >&'${mj_control_fd} EXIT
> + trap 'exit 1' INT TERM
> +}
Just wondering why $! in parent isn't used anywhere, even not for some
integrity check if the child's BASHPID actually was forked by parent.
> +multijob_post_fork() {
> + : $(( ++mj_num_jobs ))
> + if [[ ${mj_num_jobs} -ge ${mj_max_jobs} ]] ; then
> + multijob_finish_one
Feels like ignoring this child's exitstatus isn't intentional here.
> + fi
> + return 0
> +}
/haubi/
