support/scripts/fix-rpath: parallelize patch files
PATCH 1/1 support/scripts/fix-rpath: parallelize patch files
From: vdumas <[email protected]>
Using "xargs" instead of "while read" loop allows for the patching of files to be parallelized, significantly reducing the amount of time it takes to fix all the paths. On a larger RFS(~300MB) this script was taking 5 minutes, it now only takes about 30s on a 12 core machine
--- support/scripts/fix-rpath | 60 ++++++++++++++++++++++----------------- 1 file changed, 34 insertions(+), 26 deletions(-)
support/scripts/fix-rpath +34 -26
@@ -58,6 +58,38 @@ HOST_EXCLUDEPATHS="/share/terminfo" STAGING_EXCLUDEPATHS="/usr/include /usr/share/terminfo" TARGET_EXCLUDEPATHS="/lib/firmware" +patch_file() { + PATCHELF="${1}" + rootdir="${2}" + sanitize_extra_args="${3}" + file="${4}" + + # check if it's an ELF file + rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1) + if test $? -ne 0 ; then + return 0 + fi + + # make files writable if necessary + changed=$(chmod -c u+w "${file}") + + # With per-package directory support, most RPATH of host + # binaries will point to per-package directories. This won't + # work with the --make-rpath-relative ${rootdir} invocation as + # the per-package host directory is not within ${rootdir}. So, + # we rewrite all RPATHs pointing to per-package directories so + # that they point to the global host directry. + changed_rpath=$(echo ${rpath} | sed "s@${PER_PACKAGE_DIR}/[^/]\+/host@${HOST_DIR}@") + if test "${rpath}" != "${changed_rpath}" ; then + ${PATCHELF} --set-rpath ${changed_rpath} "${file}" + fi + + # call patchelf to sanitize the rpath + ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}" + # restore the original permission + test "${changed}" != "" && chmod u-w "${file}" +} + main() { local rootdir local tree="${1}"
@@ -125,32 +157,8 @@ main() { find_args+=( "-type" "f" "-print" ) - while read file ; do - # check if it's an ELF file - rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1) - if test $? -ne 0 ; then - continue - fi - - # make files writable if necessary - changed=$(chmod -c u+w "${file}") - - # With per-package directory support, most RPATH of host - # binaries will point to per-package directories. This won't - # work with the --make-rpath-relative ${rootdir} invocation as - # the per-package host directory is not within ${rootdir}. So, - # we rewrite all RPATHs pointing to per-package directories so - # that they point to the global host directry. - changed_rpath=$(echo ${rpath} | sed "s@${PER_PACKAGE_DIR}/[^/]\+/host@${HOST_DIR}@") - if test "${rpath}" != "${changed_rpath}" ; then - ${PATCHELF} --set-rpath ${changed_rpath} "${file}" - fi - - # call patchelf to sanitize the rpath - ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}" - # restore the original permission - test "${changed}" != "" && chmod u-w "${file}" - done < <(find "${rootdir}" ${find_args[@]}) + export -f patch_file + xargs -d '\n' -P $(nproc) -I {} bash -c "patch_file '${PATCHELF}' '${rootdir}' '${sanitize_extra_args}' $@" _ {} < <(find "${rootdir}" ${find_args[@]}) # Restore patched patchelf utility test "${tree}" = "host" && mv "${PATCHELF}.__to_be_patched" "${PATCHELF}"
Signature · 6 lines
2.30.2 _______________________________________________ buildroot mailing list [email protected] https://lists.buildroot.org/mailman/listinfo/buildroot
Hi Victor,
Quoted 4 lines · Victor Dumas
From: vdumas <[email protected]>
Using "xargs" instead of "while read" loop allows for the patching of files to be parallelized, significantly reducing the amount of time it takes to fix all the paths. On a larger RFS(~300MB) this script was taking 5 minutes, it now only takes about 30s on a 12 core machine
That's the kind of numbers I like to read :)
Quoted 71 lines
Signed-off-by Victor Dumas <[email protected]>--- support/scripts/fix-rpath | 60 ++++++++++++++++++++++----------------- 1 file changed, 34 insertions(+), 26 deletions(-)support/scripts/fix-rpath +32
-19@@ -58,6 +58,38 @@ HOST_EXCLUDEPATHS="/share/terminfo" STAGING_EXCLUDEPATHS="/usr/include /usr/share/terminfo" TARGET_EXCLUDEPATHS="/lib/firmware" +patch_file() { + PATCHELF="${1}" + rootdir="${2}" + sanitize_extra_args="${3}" + file="${4}" + + # check if it's an ELF file + rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1) + if test $? -ne 0 ; then + return 0 + fi + + # make files writable if necessary + changed=$(chmod -c u+w "${file}") + + # With per-package directory support, most RPATH of host + # binaries will point to per-package directories. This won't + # work with the --make-rpath-relative ${rootdir} invocation as + # the per-package host directory is not within ${rootdir}. So, + # we rewrite all RPATHs pointing to per-package directories so + # that they point to the global host directry. + changed_rpath=$(echo ${rpath} | sed "s@${PER_PACKAGE_DIR}/[^/]\+/host@${HOST_DIR}@") + if test "${rpath}" != "${changed_rpath}" ; then + ${PATCHELF} --set-rpath ${changed_rpath} "${file}" + fi + + # call patchelf to sanitize the rpath + ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}" + # restore the original permission + test "${changed}" != "" && chmod u-w "${file}" +} + main() { local rootdir local tree="${1}"@@ -125,32 +157,8 @@ main() { find_args+=( "-type" "f" "-print" ) - while read file ; do - # check if it's an ELF file - rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1) - if test $? -ne 0 ; then - continue - fi - - # make files writable if necessary - changed=$(chmod -c u+w "${file}") - - # With per-package directory support, most RPATH of host - # binaries will point to per-package directories. This won't - # work with the --make-rpath-relative ${rootdir} invocation as - # the per-package host directory is not within ${rootdir}. So, - # we rewrite all RPATHs pointing to per-package directories so - # that they point to the global host directry. - changed_rpath=$(echo ${rpath} | sed "s@${PER_PACKAGE_DIR}/[^/]\+/host@${HOST_DIR}@") - if test "${rpath}" != "${changed_rpath}" ; then - ${PATCHELF} --set-rpath ${changed_rpath} "${file}"
support/scripts/fix-rpath main() { · line 162
Quoted 10 lines
support/scripts/fix-rpath +2
-7- fi - - # call patchelf to sanitize the rpath - ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}" - # restore the original permission - test "${changed}" != "" && chmod u-w "${file}" - done < <(find "${rootdir}" ${find_args[@]}) + export -f patch_file + xargs -d '\n' -P $(nproc) -I {} bash -c "patch_file '${PATCHELF}' '${rootdir}' '${sanitize_extra_args}' $@" _ {} < <(find "${rootdir}" ${find_args[@]})
find "${rootdir}" ${find_args[@]} -print0 | xargs -0
is probably more readable and the way it's intended to be used, c.f. second example in https://www.man7.org/linux/man-pages/man1/find.1.html#EXAMPLES
Also, xargs manpage https://www.man7.org/linux/man-pages/man1/xargs.1.html#OPTIONS specifies that -P option should be used in conjunction with -L or -n to avoid spawning only one process (admittedly can't be worse than the current implementation but could be better than the suggested one).
You probably also want xargs -r in the unlikely event there's no file found by find.
Cheers,
Quentin
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
PATCH 1/1 support/scripts/fix-rpath: parallelize patching files
Using "xargs" instead of "while read" loop allows for the patching of files to be parallelized, significantly reducing the amount of time it takes to fix all the paths. On a larger RFS(~300MB) this script was taking 5 minutes, it now only takes about 30s on a 12 core machine
--- support/scripts/fix-rpath | 62 +++++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 26 deletions(-)
support/scripts/fix-rpath +36 -26
@@ -58,6 +58,40 @@ HOST_EXCLUDEPATHS="/share/terminfo" STAGING_EXCLUDEPATHS="/usr/include /usr/share/terminfo" TARGET_EXCLUDEPATHS="/lib/firmware" +patch_file() { + PATCHELF="${1}" + rootdir="${2}" + sanitize_extra_args="${3}" + file="${4}" + + echo "patch file $file" + + # check if it's an ELF file + rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1) + if test $? -ne 0 ; then + return 0 + fi + + # make files writable if necessary + changed=$(chmod -c u+w "${file}") + + # With per-package directory support, most RPATH of host + # binaries will point to per-package directories. This won't + # work with the --make-rpath-relative ${rootdir} invocation as + # the per-package host directory is not within ${rootdir}. So, + # we rewrite all RPATHs pointing to per-package directories so + # that they point to the global host directry. + changed_rpath=$(echo ${rpath} | sed "s@${PER_PACKAGE_DIR}/[^/]\+/host@${HOST_DIR}@") + if test "${rpath}" != "${changed_rpath}" ; then + ${PATCHELF} --set-rpath ${changed_rpath} "${file}" + fi + + # call patchelf to sanitize the rpath + ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}" + # restore the original permission + test "${changed}" != "" && chmod u-w "${file}" +} + main() { local rootdir local tree="${1}"
@@ -125,32 +159,8 @@ main() { find_args+=( "-type" "f" "-print" ) - while read file ; do - # check if it's an ELF file - rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1) - if test $? -ne 0 ; then - continue - fi - - # make files writable if necessary - changed=$(chmod -c u+w "${file}") - - # With per-package directory support, most RPATH of host - # binaries will point to per-package directories. This won't - # work with the --make-rpath-relative ${rootdir} invocation as - # the per-package host directory is not within ${rootdir}. So, - # we rewrite all RPATHs pointing to per-package directories so - # that they point to the global host directry. - changed_rpath=$(echo ${rpath} | sed "s@${PER_PACKAGE_DIR}/[^/]\+/host@${HOST_DIR}@") - if test "${rpath}" != "${changed_rpath}" ; then - ${PATCHELF} --set-rpath ${changed_rpath} "${file}" - fi - - # call patchelf to sanitize the rpath - ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}" - # restore the original permission - test "${changed}" != "" && chmod u-w "${file}" - done < <(find "${rootdir}" ${find_args[@]}) + export -f patch_file + find "${rootdir}" ${find_args[@]} -print0 | xargs -0 -r -d '\n' -P $(nproc) -I {} bash -c "patch_file '${PATCHELF}' '${rootdir}' '${sanitize_extra_args}' $@" _ {} # Restore patched patchelf utility test "${tree}" = "host" && mv "${PATCHELF}.__to_be_patched" "${PATCHELF}"
Signature · 6 lines
2.30.2 _______________________________________________ buildroot mailing list [email protected] https://lists.buildroot.org/mailman/listinfo/buildroot
Re: [Buildroot] [PATCH] support/scripts/fix-rpath: parallelize patching files
Hi Victor,
Please pass -v 2 to git format-patch so that the title of the mail is [PATCH v2] otherwise it gets difficult to know which version is the last one :)
Quoted 76 lines · Victor Dumas
Using "xargs" instead of "while read" loop allows for the patching of files to be parallelized, significantly reducing the amount of time it takes to fix all the paths. On a larger RFS(~300MB) this script was taking 5 minutes, it now only takes about 30s on a 12 core machine
Signed-off-by Victor Dumas <[email protected]>--- support/scripts/fix-rpath | 62 +++++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 26 deletions(-)support/scripts/fix-rpath +34
-19@@ -58,6 +58,40 @@ HOST_EXCLUDEPATHS="/share/terminfo" STAGING_EXCLUDEPATHS="/usr/include /usr/share/terminfo" TARGET_EXCLUDEPATHS="/lib/firmware" +patch_file() { + PATCHELF="${1}" + rootdir="${2}" + sanitize_extra_args="${3}" + file="${4}" + + echo "patch file $file" + + # check if it's an ELF file + rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1) + if test $? -ne 0 ; then + return 0 + fi + + # make files writable if necessary + changed=$(chmod -c u+w "${file}") + + # With per-package directory support, most RPATH of host + # binaries will point to per-package directories. This won't + # work with the --make-rpath-relative ${rootdir} invocation as + # the per-package host directory is not within ${rootdir}. So, + # we rewrite all RPATHs pointing to per-package directories so + # that they point to the global host directry. + changed_rpath=$(echo ${rpath} | sed "s@${PER_PACKAGE_DIR}/[^/]\+/host@${HOST_DIR}@") + if test "${rpath}" != "${changed_rpath}" ; then + ${PATCHELF} --set-rpath ${changed_rpath} "${file}" + fi + + # call patchelf to sanitize the rpath + ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}" + # restore the original permission + test "${changed}" != "" && chmod u-w "${file}" +} + main() { local rootdir local tree="${1}"@@ -125,32 +159,8 @@ main() { find_args+=( "-type" "f" "-print" ) - while read file ; do - # check if it's an ELF file - rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1) - if test $? -ne 0 ; then - continue - fi - - # make files writable if necessary - changed=$(chmod -c u+w "${file}") - - # With per-package directory support, most RPATH of host - # binaries will point to per-package directories. This won't - # work with the --make-rpath-relative ${rootdir} invocation as - # the per-package host directory is not within ${rootdir}. So, - # we rewrite all RPATHs pointing to per-package directories so - # that they point to the global host directry. - changed_rpath=$(echo ${rpath} | sed "s@${PER_PACKAGE_DIR}/[^/]\+/host@${HOST_DIR}@") - if test "${rpath}" != "${changed_rpath}" ; then - ${PATCHELF} --set-rpath ${changed_rpath} "${file}"
support/scripts/fix-rpath main() { · line 164
Quoted 10 lines
support/scripts/fix-rpath +2
-7- fi - - # call patchelf to sanitize the rpath - ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}" - # restore the original permission - test "${changed}" != "" && chmod u-w "${file}" - done < <(find "${rootdir}" ${find_args[@]}) + export -f patch_file + find "${rootdir}" ${find_args[@]} -print0 | xargs -0 -r -d '\n' -P $(nproc) -I {} bash -c "patch_file '${PATCHELF}' '${rootdir}' '${sanitize_extra_args}' $@" _ {}
-0 and -d '\n' are both changing the delimiter, I assume this actually does not work because -d '\n' will be parsed last (and thus used).
Actually, I think my suggestion of find "${rootdir}" ${find_args[@]} -print0 is not good either (though probably working since -print0 will be parsed after -print), we should modify find_args above the git context here to use -print0 instead of -print.
Cheers,
Quentin
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
PATCH v2 1/1 support/scripts/fix-rpath: parallelize patching files
In reply to Victor DumasUsing "xargs" instead of "while read" loop allows for the patching of files to be parallelized, significantly reducing the amount of time it takes to fix all the paths. On a larger RFS(~300MB) this script was taking 5 minutes, it now only takes about 30s on a 12 core machine
--- support/scripts/fix-rpath | 62 +++++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 26 deletions(-)
support/scripts/fix-rpath +36 -26
@@ -58,6 +58,40 @@ HOST_EXCLUDEPATHS="/share/terminfo" STAGING_EXCLUDEPATHS="/usr/include /usr/share/terminfo" TARGET_EXCLUDEPATHS="/lib/firmware" +patch_file() { + PATCHELF="${1}" + rootdir="${2}" + sanitize_extra_args="${3}" + file="${4}" + + echo "patch file $file" + + # check if it's an ELF file + rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1) + if test $? -ne 0 ; then + return 0 + fi + + # make files writable if necessary + changed=$(chmod -c u+w "${file}") + + # With per-package directory support, most RPATH of host + # binaries will point to per-package directories. This won't + # work with the --make-rpath-relative ${rootdir} invocation as + # the per-package host directory is not within ${rootdir}. So, + # we rewrite all RPATHs pointing to per-package directories so + # that they point to the global host directry. + changed_rpath=$(echo ${rpath} | sed "s@${PER_PACKAGE_DIR}/[^/]\+/host@${HOST_DIR}@") + if test "${rpath}" != "${changed_rpath}" ; then + ${PATCHELF} --set-rpath ${changed_rpath} "${file}" + fi + + # call patchelf to sanitize the rpath + ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}" + # restore the original permission + test "${changed}" != "" && chmod u-w "${file}" +} + main() { local rootdir local tree="${1}"
@@ -125,32 +159,8 @@ main() { find_args+=( "-type" "f" "-print" ) - while read file ; do - # check if it's an ELF file - rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1) - if test $? -ne 0 ; then - continue - fi - - # make files writable if necessary - changed=$(chmod -c u+w "${file}") - - # With per-package directory support, most RPATH of host - # binaries will point to per-package directories. This won't - # work with the --make-rpath-relative ${rootdir} invocation as - # the per-package host directory is not within ${rootdir}. So, - # we rewrite all RPATHs pointing to per-package directories so - # that they point to the global host directry. - changed_rpath=$(echo ${rpath} | sed "s@${PER_PACKAGE_DIR}/[^/]\+/host@${HOST_DIR}@") - if test "${rpath}" != "${changed_rpath}" ; then - ${PATCHELF} --set-rpath ${changed_rpath} "${file}" - fi - - # call patchelf to sanitize the rpath - ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}" - # restore the original permission - test "${changed}" != "" && chmod u-w "${file}" - done < <(find "${rootdir}" ${find_args[@]}) + export -f patch_file + find "${rootdir}" ${find_args[@]} -print0 | xargs -0 -r -d '\n' -P $(nproc) -I {} bash -c "patch_file '${PATCHELF}' '${rootdir}' '${sanitize_extra_args}' $@" _ {} # Restore patched patchelf utility test "${tree}" = "host" && mv "${PATCHELF}.__to_be_patched" "${PATCHELF}"
Signature · 6 lines
2.30.2 _______________________________________________ buildroot mailing list [email protected] https://lists.buildroot.org/mailman/listinfo/buildroot
PATCH v2 1/1 support/scripts/fix-rpath: parallelize patching files
In reply to Victor DumasUsing "xargs" instead of "while read" loop allows for the patching of files to be parallelized, significantly reducing the amount of time it takes to fix all the paths.On a larger RFS(~300MB) this script was taking 5 minutes, it now only takes about 30s on a 12 core machine
--- support/scripts/fix-rpath | 66 ++++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 28 deletions(-)
support/scripts/fix-rpath +38 -28
@@ -58,6 +58,40 @@ HOST_EXCLUDEPATHS="/share/terminfo" STAGING_EXCLUDEPATHS="/usr/include /usr/share/terminfo" TARGET_EXCLUDEPATHS="/lib/firmware" +patch_file() { + PATCHELF="${1}" + rootdir="${2}" + sanitize_extra_args="${3}" + file="${4}" + + echo "patch file $file" + + # check if it's an ELF file + rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1) + if test $? -ne 0 ; then + return 0 + fi + + # make files writable if necessary + changed=$(chmod -c u+w "${file}") + + # With per-package directory support, most RPATH of host + # binaries will point to per-package directories. This won't + # work with the --make-rpath-relative ${rootdir} invocation as + # the per-package host directory is not within ${rootdir}. So, + # we rewrite all RPATHs pointing to per-package directories so + # that they point to the global host directry. + changed_rpath=$(echo ${rpath} | sed "s@${PER_PACKAGE_DIR}/[^/]\+/host@${HOST_DIR}@") + if test "${rpath}" != "${changed_rpath}" ; then + ${PATCHELF} --set-rpath ${changed_rpath} "${file}" + fi + + # call patchelf to sanitize the rpath + ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}" + # restore the original permission + test "${changed}" != "" && chmod u-w "${file}" +} + main() { local rootdir local tree="${1}"
@@ -123,34 +157,10 @@ main() { ;; esac - find_args+=( "-type" "f" "-print" ) - - while read file ; do - # check if it's an ELF file - rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1) - if test $? -ne 0 ; then - continue - fi - - # make files writable if necessary - changed=$(chmod -c u+w "${file}") - - # With per-package directory support, most RPATH of host - # binaries will point to per-package directories. This won't - # work with the --make-rpath-relative ${rootdir} invocation as - # the per-package host directory is not within ${rootdir}. So, - # we rewrite all RPATHs pointing to per-package directories so - # that they point to the global host directry. - changed_rpath=$(echo ${rpath} | sed "s@${PER_PACKAGE_DIR}/[^/]\+/host@${HOST_DIR}@") - if test "${rpath}" != "${changed_rpath}" ; then - ${PATCHELF} --set-rpath ${changed_rpath} "${file}" - fi - - # call patchelf to sanitize the rpath - ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}" - # restore the original permission - test "${changed}" != "" && chmod u-w "${file}" - done < <(find "${rootdir}" ${find_args[@]}) + find_args+=( "-type" "f" "-print0" ) + + export -f patch_file + find "${rootdir}" ${find_args[@]} | xargs -0 -r -P $(nproc) -I {} bash -c "patch_file '${PATCHELF}' '${rootdir}' '${sanitize_extra_args}' $@" _ {} # Restore patched patchelf utility test "${tree}" = "host" && mv "${PATCHELF}.__to_be_patched" "${PATCHELF}"
Signature · 6 lines
2.30.2 _______________________________________________ buildroot mailing list [email protected] https://lists.buildroot.org/mailman/listinfo/buildroot
PATCH v3 1/1 support/scripts/fix-rpath: parallelize patching files
In reply to Victor DumasUsing "xargs" instead of "while read" loop allows for the patching of files to be parallelized, significantly reducing the amount of time it takes to fix all the paths.On a larger RFS(~300MB) this script was taking 5 minutes, it now only takes about 30s on a 12 core machine
--- support/scripts/fix-rpath | 66 ++++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 28 deletions(-)
support/scripts/fix-rpath +36 -28
@@ -58,6 +58,40 @@ HOST_EXCLUDEPATHS="/share/terminfo" STAGING_EXCLUDEPATHS="/usr/include /usr/share/terminfo" TARGET_EXCLUDEPATHS="/lib/firmware" +patch_file() { + PATCHELF="${1}" + rootdir="${2}" + sanitize_extra_args="${3}" + file="${4}" + + # check if it's an ELF file + rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1) + if test $? -ne 0 ; then + return 0 + fi + + # make files writable if necessary + changed=$(chmod -c u+w "${file}") + + # With per-package directory support, most RPATH of host + # binaries will point to per-package directories. This won't + # work with the --make-rpath-relative ${rootdir} invocation as + # the per-package host directory is not within ${rootdir}. So, + # we rewrite all RPATHs pointing to per-package directories so + # that they point to the global host directry. + changed_rpath=$(echo ${rpath} | sed "s@${PER_PACKAGE_DIR}/[^/]\+/host@${HOST_DIR}@") + if test "${rpath}" != "${changed_rpath}" ; then + ${PATCHELF} --set-rpath ${changed_rpath} "${file}" + fi + + # call patchelf to sanitize the rpath + ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}" + # restore the original permission + test "${changed}" != "" && chmod u-w "${file}" +} + main() { local rootdir local tree="${1}"
@@ -123,34 +157,10 @@ main() { ;; esac - find_args+=( "-type" "f" "-print" ) - - while read file ; do - # check if it's an ELF file - rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1) - if test $? -ne 0 ; then - continue - fi - - # make files writable if necessary - changed=$(chmod -c u+w "${file}") - - # With per-package directory support, most RPATH of host - # binaries will point to per-package directories. This won't - # work with the --make-rpath-relative ${rootdir} invocation as - # the per-package host directory is not within ${rootdir}. So, - # we rewrite all RPATHs pointing to per-package directories so - # that they point to the global host directry. - changed_rpath=$(echo ${rpath} | sed "s@${PER_PACKAGE_DIR}/[^/]\+/host@${HOST_DIR}@") - if test "${rpath}" != "${changed_rpath}" ; then - ${PATCHELF} --set-rpath ${changed_rpath} "${file}" - fi - - # call patchelf to sanitize the rpath - ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}" - # restore the original permission - test "${changed}" != "" && chmod u-w "${file}" - done < <(find "${rootdir}" ${find_args[@]}) + find_args+=( "-type" "f" "-print0" ) + + export -f patch_file + find "${rootdir}" ${find_args[@]} | xargs -0 -r -P $(nproc) -I {} bash -c "patch_file '${PATCHELF}' '${rootdir}' '${sanitize_extra_args}' $@" _ {} # Restore patched patchelf utility test "${tree}" = "host" && mv "${PATCHELF}.__to_be_patched" "${PATCHELF}"
Signature · 6 lines
2.30.2 _______________________________________________ buildroot mailing list [email protected] https://lists.buildroot.org/mailman/listinfo/buildroot
Re: [Buildroot] [PATCH v3] support/scripts/fix-rpath: parallelize patching files
Hi Victor,
Quoted 5 lines · Victor Dumas
Using "xargs" instead of "while read" loop allows for the patching of files to be parallelized, significantly reducing the amount of time it takes to fix all the paths.On a larger RFS(~300MB) this script was taking 5 minutes, it now only takes about 30s on a 12 core machine
I think you should split the commit message, it looks to me single really long line.
Quoted 89 lines
Signed-off-by Victor Dumas <[email protected]>--- support/scripts/fix-rpath | 66 ++++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 28 deletions(-)support/scripts/fix-rpath +28
-0@@ -58,6 +58,40 @@ HOST_EXCLUDEPATHS="/share/terminfo" STAGING_EXCLUDEPATHS="/usr/include /usr/share/terminfo" TARGET_EXCLUDEPATHS="/lib/firmware" +patch_file() { + PATCHELF="${1}" + rootdir="${2}" + sanitize_extra_args="${3}" + file="${4}" + + # check if it's an ELF file + rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1) + if test $? -ne 0 ; then + return 0 + fi + + # make files writable if necessary + changed=$(chmod -c u+w "${file}") + + # With per-package directory support, most RPATH of host + # binaries will point to per-package directories. This won't + # work with the --make-rpath-relative ${rootdir} invocation as + # the per-package host directory is not within ${rootdir}. So, + # we rewrite all RPATHs pointing to per-package directories so + # that they point to the global host directry. + changed_rpath=$(echo ${rpath} | sed "s@ ${PER_PACKAGE_DIR}/[^/]\+/host@${HOST_DIR}@") + if test "${rpath}" != "${changed_rpath}" ; then + ${PATCHELF} --set-rpath ${changed_rpath} "${file}" + fi + + # call patchelf to sanitize the rpath + ${PATCHELF} --make-rpath-relative "${rootdir}"${sanitize_extra_args[@]} "${file}"
support/scripts/fix-rpath +4
-25+ # restore the original permission + test "${changed}" != "" && chmod u-w "${file}" +} + main() { local rootdir local tree="${1}"@@ -123,34 +157,10 @@ main() { ;; esac - find_args+=( "-type" "f" "-print" ) - - while read file ; do - # check if it's an ELF file - rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1) - if test $? -ne 0 ; then - continue - fi - - # make files writable if necessary - changed=$(chmod -c u+w "${file}") - - # With per-package directory support, most RPATH of host - # binaries will point to per-package directories. This won't - # work with the --make-rpath-relative ${rootdir} invocation as - # the per-package host directory is not within ${rootdir}. So, - # we rewrite all RPATHs pointing to per-package directories so - # that they point to the global host directry. - changed_rpath=$(echo ${rpath} | sed "s@ ${PER_PACKAGE_DIR}/[^/]\+/host@${HOST_DIR}@") - if test "${rpath}" != "${changed_rpath}" ; then - ${PATCHELF} --set-rpath ${changed_rpath} "${file}" - fi - - # call patchelf to sanitize the rpath - ${PATCHELF} --make-rpath-relative "${rootdir}"${sanitize_extra_args[@]} "${file}"
support/scripts/fix-rpath +4
-3- # restore the original permission - test "${changed}" != "" && chmod u-w "${file}" - done < <(find "${rootdir}" ${find_args[@]}) + find_args+=( "-type" "f" "-print0" ) + + export -f patch_file + find "${rootdir}" ${find_args[@]} | xargs -0 -r -P $(nproc) -I {}
Quoted 3 lines
bash -c "
patch_file'${PATCHELF}' '${rootdir}' '${sanitize_extra_args}' $@" _ {}
I'm seeing here you capped the xargs with nproc, but that couldn't be always the optimal solution. On lesser powerful machines, that could suck all the core for a very long time (experimenting here on my machine ;) ) . Probably you should cap to nproc-2 for example or look for the BR2_JLEVEL or something similar.
_______________________________________________
Quoted 4 lines
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
Signature · 15 lines
Angelo Compagnucci Software Engineer [email protected] __________________________________ Amarula Solutions SRL Via le Canevare 30, 31100 Treviso, Veneto, IT T. +39 (0)42 243 5310 [email protected] www.amarulasolutions.com [`as] https://www.amarulasolutions.com|
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
PATCH v4 1/1 support/scripts/fix-rpath: parallelize patching files
In reply to Victor DumasUsing "xargs" instead of "while read" loop allows for the patching of files to be parallelized, significantly reducing the amount of time it takes to fix all the paths.On a larger RFS(~300MB) this script was taking 5 minutes, it now only takes about 30s on a 12 core machine
--- support/scripts/fix-rpath | 66 ++++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 28 deletions(-)
support/scripts/fix-rpath +38 -28
@@ -58,6 +58,38 @@ HOST_EXCLUDEPATHS="/share/terminfo" STAGING_EXCLUDEPATHS="/usr/include /usr/share/terminfo" TARGET_EXCLUDEPATHS="/lib/firmware" +patch_file() { + PATCHELF="${1}" + rootdir="${2}" + sanitize_extra_args="${3}" + file="${4}" + + # check if it's an ELF file + rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1) + if test $? -ne 0 ; then + return 0 + fi + + # make files writable if necessary + changed=$(chmod -c u+w "${file}") + + # With per-package directory support, most RPATH of host + # binaries will point to per-package directories. This won't + # work with the --make-rpath-relative ${rootdir} invocation as + # the per-package host directory is not within ${rootdir}. So, + # we rewrite all RPATHs pointing to per-package directories so + # that they point to the global host directry. + changed_rpath=$(echo ${rpath} | sed "s@${PER_PACKAGE_DIR}/[^/]\+/host@${HOST_DIR}@") + if test "${rpath}" != "${changed_rpath}" ; then + ${PATCHELF} --set-rpath ${changed_rpath} "${file}" + fi + + # call patchelf to sanitize the rpath + ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}" + # restore the original permission + test "${changed}" != "" && chmod u-w "${file}" +} + main() { local rootdir local tree="${1}"
@@ -123,34 +155,12 @@ main() { ;; esac - find_args+=( "-type" "f" "-print" ) - - while read file ; do - # check if it's an ELF file - rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1) - if test $? -ne 0 ; then - continue - fi - - # make files writable if necessary - changed=$(chmod -c u+w "${file}") - - # With per-package directory support, most RPATH of host - # binaries will point to per-package directories. This won't - # work with the --make-rpath-relative ${rootdir} invocation as - # the per-package host directory is not within ${rootdir}. So, - # we rewrite all RPATHs pointing to per-package directories so - # that they point to the global host directry. - changed_rpath=$(echo ${rpath} | sed "s@${PER_PACKAGE_DIR}/[^/]\+/host@${HOST_DIR}@") - if test "${rpath}" != "${changed_rpath}" ; then - ${PATCHELF} --set-rpath ${changed_rpath} "${file}" - fi - - # call patchelf to sanitize the rpath - ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}" - # restore the original permission - test "${changed}" != "" && chmod u-w "${file}" - done < <(find "${rootdir}" ${find_args[@]}) + find_args+=( "-type" "f" "-print0" ) + + export -f patch_file + # Limit the number of cores used + procs=$(echo -e "$(($(nproc)-2)) \n 1" | sort -n | tail -n1) + find "${rootdir}" ${find_args[@]} | xargs -0 -r -P ${procs} -I {} bash -c "patch_file '${PATCHELF}' '${rootdir}' '${sanitize_extra_args}' $@" _ {} # Restore patched patchelf utility test "${tree}" = "host" && mv "${PATCHELF}.__to_be_patched" "${PATCHELF}"
Signature · 6 lines
2.30.2 _______________________________________________ buildroot mailing list [email protected] https://lists.buildroot.org/mailman/listinfo/buildroot
PATCH v4 1/1 support/scripts/fix-rpath: parallelize patching files
In reply to Victor DumasUsing "xargs" instead of "while read" loop allows for the patching of files to be parallelized This significantly reduces the amount of time it takes to fix all the paths.
On a larger RFS(~300MB) this script was taking 5 minutes, it now only takes about 30s on a 12 core machine
--- support/scripts/fix-rpath | 66 ++++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 28 deletions(-)
support/scripts/fix-rpath +38 -28
@@ -58,6 +58,38 @@ HOST_EXCLUDEPATHS="/share/terminfo" STAGING_EXCLUDEPATHS="/usr/include /usr/share/terminfo" TARGET_EXCLUDEPATHS="/lib/firmware" +patch_file() { + PATCHELF="${1}" + rootdir="${2}" + sanitize_extra_args="${3}" + file="${4}" + + # check if it's an ELF file + rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1) + if test $? -ne 0 ; then + return 0 + fi + + # make files writable if necessary + changed=$(chmod -c u+w "${file}") + + # With per-package directory support, most RPATH of host + # binaries will point to per-package directories. This won't + # work with the --make-rpath-relative ${rootdir} invocation as + # the per-package host directory is not within ${rootdir}. So, + # we rewrite all RPATHs pointing to per-package directories so + # that they point to the global host directry. + changed_rpath=$(echo ${rpath} | sed "s@${PER_PACKAGE_DIR}/[^/]\+/host@${HOST_DIR}@") + if test "${rpath}" != "${changed_rpath}" ; then + ${PATCHELF} --set-rpath ${changed_rpath} "${file}" + fi + + # call patchelf to sanitize the rpath + ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}" + # restore the original permission + test "${changed}" != "" && chmod u-w "${file}" +} + main() { local rootdir local tree="${1}"
@@ -123,34 +155,12 @@ main() { ;; esac - find_args+=( "-type" "f" "-print" ) - - while read file ; do - # check if it's an ELF file - rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1) - if test $? -ne 0 ; then - continue - fi - - # make files writable if necessary - changed=$(chmod -c u+w "${file}") - - # With per-package directory support, most RPATH of host - # binaries will point to per-package directories. This won't - # work with the --make-rpath-relative ${rootdir} invocation as - # the per-package host directory is not within ${rootdir}. So, - # we rewrite all RPATHs pointing to per-package directories so - # that they point to the global host directry. - changed_rpath=$(echo ${rpath} | sed "s@${PER_PACKAGE_DIR}/[^/]\+/host@${HOST_DIR}@") - if test "${rpath}" != "${changed_rpath}" ; then - ${PATCHELF} --set-rpath ${changed_rpath} "${file}" - fi - - # call patchelf to sanitize the rpath - ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}" - # restore the original permission - test "${changed}" != "" && chmod u-w "${file}" - done < <(find "${rootdir}" ${find_args[@]}) + find_args+=( "-type" "f" "-print0" ) + + export -f patch_file + # Limit the number of cores used + procs=$(echo -e "$(($(nproc)-2)) \n 1" | sort -n | tail -n1) + find "${rootdir}" ${find_args[@]} | xargs -0 -r -P ${procs} -I {} bash -c "patch_file '${PATCHELF}' '${rootdir}' '${sanitize_extra_args}' $@" _ {} # Restore patched patchelf utility test "${tree}" = "host" && mv "${PATCHELF}.__to_be_patched" "${PATCHELF}"
Signature · 6 lines
2.30.2 _______________________________________________ buildroot mailing list [email protected] https://lists.buildroot.org/mailman/listinfo/buildroot
Re: [Buildroot] [PATCH v4] support/scripts/fix-rpath: parallelize patching files
Hi Victor,
Quoted 81 lines · Victor Dumas
Using "xargs" instead of "while read" loop allows for the patching of files to be parallelized This significantly reduces the amount of time it takes to fix all the paths.
On a larger RFS(~300MB) this script was taking 5 minutes, it now only takes about 30s on a 12 core machineSigned-off-by Victor Dumas <[email protected]>--- support/scripts/fix-rpath | 66 ++++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 28 deletions(-)support/scripts/fix-rpath +32
-25@@ -58,6 +58,38 @@ HOST_EXCLUDEPATHS="/share/terminfo" STAGING_EXCLUDEPATHS="/usr/include /usr/share/terminfo" TARGET_EXCLUDEPATHS="/lib/firmware" +patch_file() { + PATCHELF="${1}" + rootdir="${2}" + sanitize_extra_args="${3}" + file="${4}" + + # check if it's an ELF file + rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1) + if test $? -ne 0 ; then + return 0 + fi + + # make files writable if necessary + changed=$(chmod -c u+w "${file}") + + # With per-package directory support, most RPATH of host + # binaries will point to per-package directories. This won't + # work with the --make-rpath-relative ${rootdir} invocation as + # the per-package host directory is not within ${rootdir}. So, + # we rewrite all RPATHs pointing to per-package directories so + # that they point to the global host directry. + changed_rpath=$(echo ${rpath} | sed "s@${PER_PACKAGE_DIR}/[^/]\+/host@${HOST_DIR}@") + if test "${rpath}" != "${changed_rpath}" ; then + ${PATCHELF} --set-rpath ${changed_rpath} "${file}" + fi + + # call patchelf to sanitize the rpath + ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}" + # restore the original permission + test "${changed}" != "" && chmod u-w "${file}" +} + main() { local rootdir local tree="${1}"@@ -123,34 +155,12 @@ main() { ;; esac - find_args+=( "-type" "f" "-print" ) - - while read file ; do - # check if it's an ELF file - rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1) - if test $? -ne 0 ; then - continue - fi - - # make files writable if necessary - changed=$(chmod -c u+w "${file}") - - # With per-package directory support, most RPATH of host - # binaries will point to per-package directories. This won't - # work with the --make-rpath-relative ${rootdir} invocation as - # the per-package host directory is not within ${rootdir}. So, - # we rewrite all RPATHs pointing to per-package directories so - # that they point to the global host directry. - changed_rpath=$(echo ${rpath} | sed "s@${PER_PACKAGE_DIR}/[^/]\+/host@${HOST_DIR}@") - if test "${rpath}" != "${changed_rpath}" ; then - ${PATCHELF} --set-rpath ${changed_rpath} "${file}" - fi - - # call patchelf to sanitize the rpath - ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}"
support/scripts/fix-rpath main() { · line 164
Quoted 10 lines
support/scripts/fix-rpath +6
-3- # restore the original permission - test "${changed}" != "" && chmod u-w "${file}" - done < <(find "${rootdir}" ${find_args[@]}) + find_args+=( "-type" "f" "-print0" ) + + export -f patch_file + # Limit the number of cores used + procs=$(echo -e "$(($(nproc)-2)) \n 1" | sort -n | tail -n1) + find "${rootdir}" ${find_args[@]} | xargs -0 -r -P ${procs} -I {} bash -c "patch_file '${PATCHELF}' '${rootdir}' '${sanitize_extra_args}' $@" _ {}
I think we should use PARALLEL_JOBS here to comply with the user-requested BR2_JLEVEL (the variable is assigned at the top of package/Makefile.in). It is used for all packages and also rootfs compression algorithms, so it would make sense to me to have this fix-rpath script respect it too.
I assume in order to pass this makefile option to the script, we'll need to add PARALLEL_JOBS=$(PARALLEL_JOBS) in front of each fix-rpath script? Similar to what's been done for PER_PACKAGE_DIR in Makefile.
I can suggest the following (to apply on top of the current patch and squash if that works for you):
```
diff --git a/Makefile b/Makefile
index 7c1c07a2e4..c171aa94de 100644
--- a/Makefile
+++ b/Makefile
@@ -593,8 +593,8 @@ world: target-post-image
.PHONY: prepare-sdk
prepare-sdk: world
@$(call MESSAGE,"Rendering the SDK relocatable")
- PER_PACKAGE_DIR=$(PER_PACKAGE_DIR) $(TOPDIR)/support/scripts/fix-rpath host
- PER_PACKAGE_DIR=$(PER_PACKAGE_DIR) $(TOPDIR)/support/scripts/fix-rpath staging
+ PARALLEL_JOBS=$(PARALLEL_JOBS) PER_PACKAGE_DIR=$(PER_PACKAGE_DIR) $(TOPDIR)/support/scripts/fix-rpath host
+ PARALLEL_JOBS=$(PARALLEL_JOBS) PER_PACKAGE_DIR=$(PER_PACKAGE_DIR) $(TOPDIR)/support/scripts/fix-rpath staging
$(INSTALL) -m 755 $(TOPDIR)/support/misc/relocate-sdk.sh
$(HOST_DIR)/relocate-sdk.sh
mkdir -p $(HOST_DIR)/share/buildroot
echo $(HOST_DIR) > $(HOST_DIR)/share/buildroot/sdk-location
@@ -764,7 +764,7 @@ endif
ln -sf ../usr/lib/os-release $(TARGET_DIR)/etc
@$(call MESSAGE,"Sanitizing RPATH in target tree")
- PER_PACKAGE_DIR=$(PER_PACKAGE_DIR) $(TOPDIR)/support/scripts/fix-rpath target
+ PARALLEL_JOBS=$(PARALLEL_JOBS) PER_PACKAGE_DIR=$(PER_PACKAGE_DIR) $(TOPDIR)/support/scripts/fix-rpath target
# For a merged /usr, ensure that /lib, /bin and /sbin and their /usr
# counterparts are appropriately setup as symlinks ones to the others.
diff --git a/support/scripts/fix-rpath b/support/scripts/fix-rpath
index cef7a48e76..6160efd461 100755
--- a/support/scripts/fix-rpath
+++ b/support/scripts/fix-rpath
@@ -158,9 +158,7 @@ main() {
find_args+=( "-type" "f" "-print0" )
export -f patch_file
- # Limit the number of cores used
- procs=$(echo -e "$(($(nproc)-2)) \n 1" | sort -n | tail -n1)
- find "${rootdir}" ${find_args[@]} | xargs -0 -r -P ${procs} -I {} bash -c "patch_file '${PATCHELF}' '${rootdir}' '${sanitize_extra_args}' $@" _ {}
+ find "${rootdir}" ${find_args[@]} | xargs -0 -r -P ${PARALLEL_JOBS} -I {} bash -c "patch_file '${PATCHELF}' '${rootdir}' '${sanitize_extra_args}' $@" _ {}
# Restore patched patchelf utility
test "${tree}" = "host" && mv "${PATCHELF}.__to_be_patched"
"${PATCHELF}"
```I tested on a slow-ish laptop (Intel(R) Core(TM) i7-10510U CPU @ 1.80GHz 4c/8t) with:
make raspberrypi4_defconfig
sudo sh -c "echo 3 > /proc/sys/vm/drop_caches"
time make target-finalize
# apply patch + diff above sudo sh -c "echo 3 > /proc/sys/vm/drop_caches" time make target-finalize
Before:
make target-finalize 8.23s user 3.52s system 91% cpu 12.851 total After:
make target-finalize 9.25s user 4.81s system 158% cpu 8.892 total
Cheers,
Quentin
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
Re: [Buildroot] [PATCH v4] support/scripts/fix-rpath: parallelize patching files
In reply to Victor DumasFrom: Victor Dumas
Quoted 6 lines
Sent: 20 October 2022 12:55
Using "xargs" instead of "while read" loop allows for the patching of files to be parallelized This significantly reduces the amount of time it takes to fix all the paths.
On a larger RFS(~300MB) this script was taking 5 minutes, it now only takes about 30s on a 12 core machine
...
Quoted 4 lines
support/scripts/fix-rpath +3
-0+ # Limit the number of cores used + procs=$(echo -e "$(($(nproc)-2)) \n 1" | sort -n | tail -n1) + find "${rootdir}" ${find_args[@]} | xargs -0 -r -P ${procs} -I {} bash -c "patch_file'${PATCHELF}' '${rootdir}' '${sanitize_extra_args}' $@" _ {}
Are you sure that all versions of xargs the script needs to run on support -P ?
Having xargs run 'bash -c "command" ...' and the quoting also looks odd. I presume there is some reason xargs can't just run patrch_file.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
Re: [Buildroot] [PATCH v4] support/scripts/fix-rpath: parallelize patching files
Hi David,
Quoted 7 lines · David Laight
From: Victor Dumas
Quoted 6 lines
Sent: 20 October 2022 12:55
Using "xargs" instead of "while read" loop allows for the patching of files to be parallelized This significantly reduces the amount of time it takes to fix all the paths.
On a larger RFS(~300MB) this script was taking 5 minutes, it now only takes about 30s on a 12 core machine
Quoted 9 lines
...
Quoted 4 lines
support/scripts/fix-rpath +3
-0+ # Limit the number of cores used + procs=$(echo -e "$(($(nproc)-2)) \n 1" | sort -n | tail -n1) + find "${rootdir}" ${find_args[@]} | xargs -0 -r -P ${procs} -I {} bash -c "patch_file'${PATCHELF}' '${rootdir}' '${sanitize_extra_args}' $@" _ {}
Are you sure that all versions of xargs the script needs to run on support -P ?
--max-procs was part of the initial commit of xargs in findutils in 1996, c.f.:
https://git.savannah.gnu.org/cgit/findutils.git/commit/?id=c030b5ee33bbec3c93cddc3ca9ebec14c24dbe07
It is also supported in its initial commit in Busybox:
https://git.busybox.net/busybox/commit/?id=92a61c1206572f4a6e55baa24e7cdd4f180d4b64
The only question is about BSD support (since pkgs.org returns that all other distros are using find-utils/busybox basically) since they are using something else (toybox, heirloom, ...?).
Do we actually have a list of operating systems that are "officially" supported?
Quoted 3 lines
Having xargs run 'bash -c "command" ...' and the quoting also looks odd. I presume there is some reason xargs can't just run
patrch_file.
IIRC, you can't call a shell function from xargs directly.
Cheers,
Quentin
Quoted 10 lines
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)_______________________________________________
buildroot mailing list
[email protected]
https://urldefense.com/v3/__https://lists.buildroot.org/mailman/listinfo/buildroot__;!!OOPJP91ZZw!mrCuxJUbX9qAxmtZDc6047Tt9_CuDKxXgPQu86pqCMlg_9auvbbJCmj2U4UdX_OnaO0U9ecmUu0-uHnkJjFiEXoSX0FyA8U_YtvT0A$
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
Re: [Buildroot] [PATCH v4] support/scripts/fix-rpath: parallelize patching files
Rather than doing it via a shell script and xargs -P, consider doing parallelization in a submake, similar to how we handle parallel glibc locale generation:
https://github.com/buildroot/buildroot/blob/master/support/misc/gen-glibc-locales.mk
https://github.com/buildroot/buildroot/blob/bc9b716296f37e0e3e47fd34c8991e92b6baeebd/Makefile#L655-L656
This approach will reuse the make job server.
You'd need to assign the results of the find to a variable and then create a target for each of the files using something like
$(foreach file,$(FILES),$(eval $(call make-fix-rpath-target,$(file))))
Quoted 60 lines · Quentin Schulz via buildroot
Hi David,
Quoted 4 lines · David Laight
From: Victor Dumas
Quoted 3 lines
Sent: 20 October 2022 12:55
Using "xargs" instead of "while read" loop allows for the patching of
files to be parallelized
Quoted 1 line
Quoted 1 line
This significantly reduces the amount of time it takes to fix all the
paths.
Quoted 1 line
Quoted 1 line
On a larger RFS(~300MB) this script was taking 5 minutes, it now only
takes about 30s on a 12 core
Quoted 5 lines
Quoted 1 line
machine
...
support/scripts/fix-rpath main() {
Quoted 3 lines
support/scripts/fix-rpath +3
-0+ # Limit the number of cores used + procs=$(echo -e "$(($(nproc)-2)) \n 1" | sort -n | tail -n1) + find "${rootdir}" ${find_args[@]} | xargs -0 -r -P ${procs} -I {}bash -c "
patch_fileQuoted 5 lines
Quoted 1 line
'${PATCHELF}' '${rootdir}' '${sanitize_extra_args}' $@" _ {}
Are you sure that all versions of xargs the script needs to run on support -P ?
--max-procs was part of the initial commit of xargs in findutils in 1996, c.f.:
https://git.savannah.gnu.org/cgit/findutils.git/commit/?id=c030b5ee33bbec3c93cddc3ca9ebec14c24dbe07
It is also supported in its initial commit in Busybox:
https://git.busybox.net/busybox/commit/?id=92a61c1206572f4a6e55baa24e7cdd4f180d4b64
The only question is about BSD support (since pkgs.org returns that all other distros are using find-utils/busybox basically) since they are using something else (toybox, heirloom, ...?).
Do we actually have a list of operating systems that are "officially" supported?
Quoted 3 lines
Having xargs run 'bash -c "command" ...' and the quoting also looks odd. I presume there is some reason xargs can't just run
patrch_file.IIRC, you can't call a shell function from xargs directly.
Cheers,
QuentinQuoted 4 lines
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes,MK1 1PT, UK
Quoted 6 lines
Registration No: 1397386 (Wales)
_______________________________________________
buildroot mailing list
[email protected]
Quoted 6 lines
https://urldefense.com/v3/__https://lists.buildroot.org/mailman/listinfo/buildroot__;!!OOPJP91ZZw!mrCuxJUbX9qAxmtZDc6047Tt9_CuDKxXgPQu86pqCMlg_9auvbbJCmj2U4UdX_OnaO0U9ecmUu0-uHnkJjFiEXoSX0FyA8U_YtvT0A$
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
PATCH v4 1/1 support/scripts/fix-rpath: parallelize patching files
In reply to Victor DumasUsing "xargs" instead of "while read" loop allows for the patching of files to be parallelized. This significantly reduces the amount of time it takes to fix all the paths.
On a larger RFS(~300MB) this script was taking 5 minutes, it now only takes about 30s on a 12 core machine
--- support/scripts/fix-rpath | 66 ++++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 28 deletions(-)
support/scripts/fix-rpath +38 -28
@@ -58,6 +58,40 @@ HOST_EXCLUDEPATHS="/share/terminfo" STAGING_EXCLUDEPATHS="/usr/include /usr/share/terminfo" TARGET_EXCLUDEPATHS="/lib/firmware" +patch_file() { + PATCHELF="${1}" + rootdir="${2}" + sanitize_extra_args="${3}" + file="${4}" + + # check if it's an ELF file + rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1) + if test $? -ne 0 ; then + return 0 + fi + + # make files writable if necessary + changed=$(chmod -c u+w "${file}") + + # With per-package directory support, most RPATH of host + # binaries will point to per-package directories. This won't + # work with the --make-rpath-relative ${rootdir} invocation as + # the per-package host directory is not within ${rootdir}. So, + # we rewrite all RPATHs pointing to per-package directories so + # that they point to the global host directry. + changed_rpath=$(echo ${rpath} | sed "s@${PER_PACKAGE_DIR}/[^/]\+/host@${HOST_DIR}@") + if test "${rpath}" != "${changed_rpath}" ; then + ${PATCHELF} --set-rpath ${changed_rpath} "${file}" + fi + + # call patchelf to sanitize the rpath + ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}" + # restore the original permission + test "${changed}" != "" && chmod u-w "${file}" +} + main() { local rootdir local tree="${1}"
@@ -123,34 +157,10 @@ main() { ;; esac - find_args+=( "-type" "f" "-print" ) - - while read file ; do - # check if it's an ELF file - rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1) - if test $? -ne 0 ; then - continue - fi - - # make files writable if necessary - changed=$(chmod -c u+w "${file}") - - # With per-package directory support, most RPATH of host - # binaries will point to per-package directories. This won't - # work with the --make-rpath-relative ${rootdir} invocation as - # the per-package host directory is not within ${rootdir}. So, - # we rewrite all RPATHs pointing to per-package directories so - # that they point to the global host directry. - changed_rpath=$(echo ${rpath} | sed "s@${PER_PACKAGE_DIR}/[^/]\+/host@${HOST_DIR}@") - if test "${rpath}" != "${changed_rpath}" ; then - ${PATCHELF} --set-rpath ${changed_rpath} "${file}" - fi - - # call patchelf to sanitize the rpath - ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}" - # restore the original permission - test "${changed}" != "" && chmod u-w "${file}" - done < <(find "${rootdir}" ${find_args[@]}) + find_args+=( "-type" "f" "-print0" ) + + export -f patch_file + # Limit the number of cores used + procs=$(echo -e "$(($(nproc)-2)) \n 1" | sort -n | tail -n1) + find "${rootdir}" ${find_args[@]} | xargs -0 -r -P ${procs} -I {} bash -c "patch_file '${PATCHELF}' '${rootdir}' '${sanitize_extra_args}' $@" _ {}
# Restore patched patchelf utility
test "${tree}" = "host" && mv "${PATCHELF}.__to_be_patched" "${PATCHELF}"
Signature · 6 lines
2.30.2 _______________________________________________ buildroot mailing list [email protected] https://lists.buildroot.org/mailman/listinfo/buildroot
PATCH v4 1/1 support/scripts/fix-rpath: parallelize patching files
In reply to Victor DumasUsing "xargs" instead of "while read" loop allows for the patching of files to be parallelized. This significantly reduces the amount of time it takes to fix all the paths.
On a larger RFS(~300MB) this script was taking 5 minutes, it now only takes about 30s on a 12 core machine
--- support/scripts/fix-rpath | 66 ++++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 28 deletions(-)
support/scripts/fix-rpath +38 -28
@@ -58,6 +58,40 @@ HOST_EXCLUDEPATHS="/share/terminfo" STAGING_EXCLUDEPATHS="/usr/include /usr/share/terminfo" TARGET_EXCLUDEPATHS="/lib/firmware" +patch_file() { + PATCHELF="${1}" + rootdir="${2}" + sanitize_extra_args="${3}" + file="${4}" + + # check if it's an ELF file + rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1) + if test $? -ne 0 ; then + return 0 + fi + + # make files writable if necessary + changed=$(chmod -c u+w "${file}") + + # With per-package directory support, most RPATH of host + # binaries will point to per-package directories. This won't + # work with the --make-rpath-relative ${rootdir} invocation as + # the per-package host directory is not within ${rootdir}. So, + # we rewrite all RPATHs pointing to per-package directories so + # that they point to the global host directry. + changed_rpath=$(echo ${rpath} | sed "s@${PER_PACKAGE_DIR}/[^/]\+/host@${HOST_DIR}@") + if test "${rpath}" != "${changed_rpath}" ; then + ${PATCHELF} --set-rpath ${changed_rpath} "${file}" + fi + + # call patchelf to sanitize the rpath + ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}" + # restore the original permission + test "${changed}" != "" && chmod u-w "${file}" +} + main() { local rootdir local tree="${1}"
@@ -123,34 +155,12 @@ main() { ;; esac - find_args+=( "-type" "f" "-print" ) - - while read file ; do - # check if it's an ELF file - rpath=$(${PATCHELF} --print-rpath "${file}" 2>&1) - if test $? -ne 0 ; then - continue - fi - - # make files writable if necessary - changed=$(chmod -c u+w "${file}") - - # With per-package directory support, most RPATH of host - # binaries will point to per-package directories. This won't - # work with the --make-rpath-relative ${rootdir} invocation as - # the per-package host directory is not within ${rootdir}. So, - # we rewrite all RPATHs pointing to per-package directories so - # that they point to the global host directry. - changed_rpath=$(echo ${rpath} | sed "s@${PER_PACKAGE_DIR}/[^/]\+/host@${HOST_DIR}@") - if test "${rpath}" != "${changed_rpath}" ; then - ${PATCHELF} --set-rpath ${changed_rpath} "${file}" - fi - - # call patchelf to sanitize the rpath - ${PATCHELF} --make-rpath-relative "${rootdir}" ${sanitize_extra_args[@]} "${file}" - # restore the original permission - test "${changed}" != "" && chmod u-w "${file}" - done < <(find "${rootdir}" ${find_args[@]}) + find_args+=( "-type" "f" "-print0" ) + + export -f patch_file + # Limit the number of cores used + procs=$(echo -e "$(($(nproc)-2)) \n 1" | sort -n | tail -n1) + find "${rootdir}" ${find_args[@]} | xargs -0 -r -P ${procs} -I {} bash -c "patch_file '${PATCHELF}' '${rootdir}' '${sanitize_extra_args}' $@" _ {} # Restore patched patchelf utility test "${tree}" = "host" && mv "${PATCHELF}.__to_be_patched" "${PATCHELF}"
Signature · 6 lines
2.30.2 _______________________________________________ buildroot mailing list [email protected] https://lists.buildroot.org/mailman/listinfo/buildroot
Thomas Petazzoni via buildroot
AppliedRe: [Buildroot] [PATCH v4] support/scripts/fix-rpath: parallelize patching files
Quoted 8 lines · On Thu, 20 Oct 2022 13:50:30 +0200 Victor Dumas <[email protected]> wrote:
Using "xargs" instead of "while read" loop allows for the patching of files to be parallelized. This significantly reduces the amount of time it takes to fix all the paths.
On a larger RFS(~300MB) this script was taking 5 minutes, it now only takes about 30s on a 12 core machineSigned-off-by Victor Dumas <[email protected]>--- support/scripts/fix-rpath | 66 ++++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 28 deletions(-)
Applied to next after taking into account the feedback from Quentin Schulz on using $(PARALLEL_JOBS) to specify the number of cores.
Thanks!
Thomas
Signature · 7 lines
Thomas Petazzoni, CTO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com _______________________________________________ buildroot mailing list [email protected] https://lists.buildroot.org/mailman/listinfo/buildroot