From 572ae53676f892a1570bbb75ae2f7cefbb1f7fe7 Mon Sep 17 00:00:00 2001 From: Eric Miotto <1094986+edymtt@users.noreply.github.com> Date: Mon, 16 Nov 2020 14:20:08 -0800 Subject: [PATCH 1/4] [build-script] Allow to tune dsymutil parallelism This should enable scaling when using machines with large amount of RAM. To better support machines with lower spec, process one binary per dsymutil invocation (reverting #34149). Add some (limited) facilities to gather the time taken to execute dsymutil to better assist in tuning the parameter. Addresses rdar://71018443 --- utils/build-script | 1 + utils/build-script-impl | 31 +++++++++++++++++-- utils/build_swift/build_swift/defaults.py | 5 ++- .../build_swift/driver_arguments.py | 7 +++++ utils/build_swift/tests/expected_options.py | 2 ++ .../BuildSystem/dsymutil_jobs.test | 10 ++++++ 6 files changed, 53 insertions(+), 3 deletions(-) create mode 100644 validation-test/BuildSystem/dsymutil_jobs.test diff --git a/utils/build-script b/utils/build-script index 8b1c1cd4fde60..3a34ceb487046 100755 --- a/utils/build-script +++ b/utils/build-script @@ -499,6 +499,7 @@ class BuildScriptInvocation(object): pipes.quote(opt) for opt in cmake.common_options()), "--build-args=%s" % ' '.join( pipes.quote(arg) for arg in cmake.build_args()), + "--dsymutil-jobs", str(args.dsymutil_jobs), ] # Compute any product specific cmake arguments. diff --git a/utils/build-script-impl b/utils/build-script-impl index 5613d5beb4835..eb0da251ea964 100755 --- a/utils/build-script-impl +++ b/utils/build-script-impl @@ -71,6 +71,7 @@ KNOWN_SETTINGS=( swift-tools-num-parallel-lto-link-jobs "" "The number of parallel link jobs to use when compiling swift tools" use-gold-linker "" "Enable using the gold linker" workspace "${HOME}/src" "source directory containing llvm, clang, swift" + dsymutil-jobs "1" "number of parallel invocations of dsymutil" ## Build Tools host-cc "" "the path to CC, the 'clang' compiler for the host platform. **This argument is required**" @@ -3015,6 +3016,25 @@ for host in "${ALL_HOSTS[@]}"; do done done +function printJSONTimestamp { + local command=$1 + local kind=$2 + + echo "{ \"command\": \"${command}\", \"${kind}\": \"$(date "+%Y-%m-%dT%H:%M:%S")\" }" +} + +function printJSONStartTimestamp { + local command=$1 + + printJSONTimestamp ${command} "start" +} + +function printJSONEndTimestamp { + local command=$1 + + printJSONTimestamp ${command} "end" +} + for host in "${ALL_HOSTS[@]}"; do # Check if we should perform this action. if ! [[ $(should_execute_action "${host}-extractsymbols") ]]; then @@ -3030,6 +3050,7 @@ for host in "${ALL_HOSTS[@]}"; do host_install_destdir=$(get_host_install_destdir ${host}) host_install_prefix=$(get_host_install_prefix ${host}) + if [[ "${DARWIN_INSTALL_EXTRACT_SYMBOLS}" ]] && [[ $(host_has_darwin_symbols ${host}) ]]; then echo "--- Extracting symbols ---" @@ -3047,6 +3068,9 @@ for host in "${ALL_HOSTS[@]}"; do # Instead, just echo we do "darwin_intall_extract_symbols". if [[ "${DRY_RUN}" ]]; then call darwin_install_extract_symbols + printJSONStartTimestamp dsymutil + echo xargs -n 1 -P ${DSYMUTIL_JOBS} dsymutil + printJSONEndTimestamp dsymutil else set -x @@ -3069,13 +3093,16 @@ for host in "${ALL_HOSTS[@]}"; do # # Exclude shell scripts and static archives. # Exclude swift-api-digester dSYM to reduce debug toolchain size. - # Run sequentially -- dsymutil is multithreaded and can be memory intensive + # Tweak carefully the amount of parallelism -- dsymutil can be memory intensive and + # as such too many instance can exhaust the memory and slow down/panic the machine + printJSONStartTimestamp dsymutil (cd "${host_symroot}" && find ./"${CURRENT_PREFIX}" -perm -0111 -type f -print | \ grep -v '.py$' | \ grep -v '.a$' | \ grep -v 'swift-api-digester' | \ - xargs -P 1 ${dsymutil_path}) + xargs -n 1 -P ${DSYMUTIL_JOBS} ${dsymutil_path}) + printJSONEndTimestamp dsymutil # Strip executables, shared libraries and static libraries in # `host_install_destdir`. diff --git a/utils/build_swift/build_swift/defaults.py b/utils/build_swift/build_swift/defaults.py index 86beac7bb44bb..db1476df9e894 100644 --- a/utils/build_swift/build_swift/defaults.py +++ b/utils/build_swift/build_swift/defaults.py @@ -22,7 +22,7 @@ __all__ = [ - # Command line configuarable + # Command line configurable 'BUILD_VARIANT', 'CMAKE_GENERATOR', 'COMPILER_VENDOR', @@ -38,6 +38,7 @@ 'DARWIN_INSTALL_PREFIX', 'LLVM_MAX_PARALLEL_LTO_LINK_JOBS', 'SWIFT_MAX_PARALLEL_LTO_LINK_JOBS', + 'DSYMUTIL_JOBS' # Constants ] @@ -62,6 +63,8 @@ DARWIN_INSTALL_PREFIX = ('/Applications/Xcode.app/Contents/Developer/' 'Toolchains/XcodeDefault.xctoolchain/usr') +DSYMUTIL_JOBS = 1 + def _system_memory(): """Returns the system memory as an int. None if the system memory cannot diff --git a/utils/build_swift/build_swift/driver_arguments.py b/utils/build_swift/build_swift/driver_arguments.py index 45d76b5195178..28a3ec0e6ab26 100644 --- a/utils/build_swift/build_swift/driver_arguments.py +++ b/utils/build_swift/build_swift/driver_arguments.py @@ -486,6 +486,13 @@ def create_argument_parser(): help='the maximum number of parallel link jobs to use when ' 'compiling swift tools.') + option('--dsymutil-jobs', store_int, + default=defaults.DSYMUTIL_JOBS, + metavar='COUNT', + help='the maximum number of parallel dsymutil jobs to use when ' + 'extracting symbols. Tweak with caution, since dsymutil' + 'is memory intensive.') + option('--disable-guaranteed-normal-arguments', store_true, help='Disable guaranteed normal arguments') diff --git a/utils/build_swift/tests/expected_options.py b/utils/build_swift/tests/expected_options.py index fd1820ce9cf5f..8f13cca65b9c4 100644 --- a/utils/build_swift/tests/expected_options.py +++ b/utils/build_swift/tests/expected_options.py @@ -140,6 +140,7 @@ 'distcc': False, 'sccache': False, 'dry_run': False, + 'dsymutil_jobs': defaults.DSYMUTIL_JOBS, 'enable_asan': False, 'enable_experimental_differentiable_programming': True, 'enable_experimental_concurrency': True, @@ -662,6 +663,7 @@ class BuildScriptImplOption(_BaseOption): IntOption('--llvm-max-parallel-lto-link-jobs'), IntOption('--swift-tools-max-parallel-lto-link-jobs'), IntOption('-j', dest='build_jobs'), + IntOption('--dsymutil-jobs', dest='dsymutil_jobs'), AppendOption('--cross-compile-hosts'), AppendOption('--extra-cmake-options'), diff --git a/validation-test/BuildSystem/dsymutil_jobs.test b/validation-test/BuildSystem/dsymutil_jobs.test new file mode 100644 index 0000000000000..5fad5bceec922 --- /dev/null +++ b/validation-test/BuildSystem/dsymutil_jobs.test @@ -0,0 +1,10 @@ +# RUN: %empty-directory(%t) +# RUN: mkdir -p %t +# RUN: SKIP_XCODE_VERSION_CHECK=1 SWIFT_BUILD_ROOT=%t %swift_src_root/utils/build-script --dry-run --darwin-install-extract-symbols --dsymutil-jobs 5 --cmake %cmake 2>&1 | %FileCheck %s + +# REQUIRES: standalone_build + +# CHECK: --- Extracting symbols --- +# CHECK: { "command": "dsymutil", "start": " +# CHECK-NEXT: xargs -n 1 -P 5 dsymutil +# CHECK-NEXT: { "command": "dsymutil", "end": " From 0d0199fe24942fae15c674179e0a97ee8f67656e Mon Sep 17 00:00:00 2001 From: Eric Miotto <1094986+edymtt@users.noreply.github.com> Date: Wed, 18 Nov 2020 15:52:39 -0800 Subject: [PATCH 2/4] Restrict test to macOS --- validation-test/BuildSystem/dsymutil_jobs.test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/validation-test/BuildSystem/dsymutil_jobs.test b/validation-test/BuildSystem/dsymutil_jobs.test index 5fad5bceec922..d20190363a36f 100644 --- a/validation-test/BuildSystem/dsymutil_jobs.test +++ b/validation-test/BuildSystem/dsymutil_jobs.test @@ -2,7 +2,7 @@ # RUN: mkdir -p %t # RUN: SKIP_XCODE_VERSION_CHECK=1 SWIFT_BUILD_ROOT=%t %swift_src_root/utils/build-script --dry-run --darwin-install-extract-symbols --dsymutil-jobs 5 --cmake %cmake 2>&1 | %FileCheck %s -# REQUIRES: standalone_build +# REQUIRES: standalone_build,OS=macosx # CHECK: --- Extracting symbols --- # CHECK: { "command": "dsymutil", "start": " From 17388de4048a6da7160ff1c82d87ae3312e5f7be Mon Sep 17 00:00:00 2001 From: Eric Miotto <1094986+edymtt@users.noreply.github.com> Date: Wed, 18 Nov 2020 15:53:04 -0800 Subject: [PATCH 3/4] Use bash style definition for timestamp functions --- utils/build-script-impl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/utils/build-script-impl b/utils/build-script-impl index eb0da251ea964..d3d53475530cd 100755 --- a/utils/build-script-impl +++ b/utils/build-script-impl @@ -3016,20 +3016,20 @@ for host in "${ALL_HOSTS[@]}"; do done done -function printJSONTimestamp { +function printJSONTimestamp() { local command=$1 local kind=$2 echo "{ \"command\": \"${command}\", \"${kind}\": \"$(date "+%Y-%m-%dT%H:%M:%S")\" }" } -function printJSONStartTimestamp { +function printJSONStartTimestamp() { local command=$1 printJSONTimestamp ${command} "start" } -function printJSONEndTimestamp { +function printJSONEndTimestamp() { local command=$1 printJSONTimestamp ${command} "end" From a77ce30d25daeb3967ed0dc35153140e9125f127 Mon Sep 17 00:00:00 2001 From: Eric Miotto <1094986+edymtt@users.noreply.github.com> Date: Thu, 19 Nov 2020 08:21:54 -0800 Subject: [PATCH 4/4] Remove unnecessary spacing --- utils/build-script-impl | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/utils/build-script-impl b/utils/build-script-impl index d3d53475530cd..78fcb5d05f5eb 100755 --- a/utils/build-script-impl +++ b/utils/build-script-impl @@ -3020,7 +3020,7 @@ function printJSONTimestamp() { local command=$1 local kind=$2 - echo "{ \"command\": \"${command}\", \"${kind}\": \"$(date "+%Y-%m-%dT%H:%M:%S")\" }" + echo "{ \"command\": \"${command}\", \"${kind}\": \"$(date "+%Y-%m-%dT%H:%M:%S")\" }" } function printJSONStartTimestamp() { @@ -3050,7 +3050,6 @@ for host in "${ALL_HOSTS[@]}"; do host_install_destdir=$(get_host_install_destdir ${host}) host_install_prefix=$(get_host_install_prefix ${host}) - if [[ "${DARWIN_INSTALL_EXTRACT_SYMBOLS}" ]] && [[ $(host_has_darwin_symbols ${host}) ]]; then echo "--- Extracting symbols ---"