Skip to content

Reduce configure times when regenerating clients#3765

Open
apache-hb wants to merge 1 commit intoaws:mainfrom
apache-hb:parallel-clang-format
Open

Reduce configure times when regenerating clients#3765
apache-hb wants to merge 1 commit intoaws:mainfrom
apache-hb:parallel-clang-format

Conversation

@apache-hb
Copy link
Copy Markdown
Contributor

Description of changes:

Runs clang-format in parallel after generating client code. On my epyc 9654 this reduces the time to configure a new build directory by about ~75%. The implementation is a bit naive and only splits the generated file set into equal sizes, when regenerating a single client some clang-format processess will only format a single file.

# build.sh
cmake -S . -B $BUILDDIR \
    -DAUTORUN_UNIT_TESTS=OFF \
    -DREGENERATE_CLIENTS=ON \
    -DCMAKE_BUILD_TYPE=MinSizeRel \
    -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
    -DCMAKE_INSTALL_PREFIX=$INSTALLDIR \
    -GNinja
elliothb@ELLIOT-SERVER:~/github/aws-sdk-cpp-parallel-clang-format$ time ./build.sh build-before $(pwd)/install-before > before.log 2>&1

real    12m16.217s
user    143m26.334s
sys     15m36.796s
elliothb@ELLIOT-SERVER:~/github/aws-sdk-cpp-parallel-clang-format$ time ./build.sh build-after $(pwd)/install-after > after.log 2>&1

real    2m45.729s
user    141m31.590s
sys     16m12.126s

I think the only mild downside is the logs look a bit uglier as they now contain interleaved invocations of clang-format

Clang-formating 782 files
Clang-formating 782 files
⚠️  clang-format is already on your PATH and installed at
    /usr/local/bin/clang-format. Downloading and running anyway.
Clang-formating 782 files
⚠️  clang-format is already on your PATH and installed at
    /usr/local/bin/clang-format. Downloading and running anyway.
⚠️  clang-format is already on your PATH and installed at
    /usr/local/bin/clang-format. Downloading and running anyway.
Clang-formating 782 files
Clang-formating 782 files
⚠️  clang-format is already on your PATH and installed at
    /usr/local/bin/clang-format. Downloading and running anyway.

Check all that applies:

  • Did a review by yourself.
  • Added proper tests to cover this PR. (If tests are not applicable, explain.)
  • Checked if this PR is a breaking (APIs have been changed) change.
  • Checked if this PR will not introduce cross-platform inconsistent behavior.
  • Checked if this PR would require a ReadMe/Wiki update.

Check which platforms you have built SDK on to verify the correctness of this PR.

  • Linux
  • Windows
  • Android
  • MacOS
  • IOS
  • Other Platforms

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant