Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance "actuallyCompleteAction" algorithm for better build performance #18614

Open
chiragramani opened this issue Jun 8, 2023 · 5 comments
Open
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Remote-Exec Issues and PRs for the Execution (Remote) team type: bug

Comments

@chiragramani
Copy link
Contributor

chiragramani commented Jun 8, 2023

Description of the bug:

actuallyCompleteAction adversely affects our build performance and has been a constant bottleneck.

As we analyze the step named 'Bundling, processing and signing App`, it is primarily composed of 2 parts.

  1. BundleTreeApp: 272ms
  2. actuallyCompleteAction: 12s 825ms

We can see that almost 100% of the time of this step is the overhead introduced by actuallyCompleteAction.
Screenshot 2023-06-07 at 6 51 37 PM

This happens on every build and comes with a significantly high wait time cost as artifacts are huge. Any improvements here would be super helpful.

Related threads: #18194, #17009, #10702

Below is a repro that demonstrates the bottleneck.

Repro on macOS:

bazel build //:App

test_bundling2.zip

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Repro on macOS:

bazel build //:App

test_bundling2.zip

Which operating system are you running Bazel on?

macOS

What is the output of bazel info release?

release 6.2.1

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Is this a regression? If yes, please try to identify the Bazel commit where the bug was introduced.

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

@brentleyjones
Copy link
Contributor

@tylerwilliams

@Pavank1992 Pavank1992 added the team-Remote-Exec Issues and PRs for the Execution (Remote) team label Jun 8, 2023
@Pavank1992 Pavank1992 removed their assignment Jun 8, 2023
@coeuvre coeuvre added P2 We'll consider working on this in future. (Assignee optional) and removed untriaged labels Jun 13, 2023
@brentleyjones
Copy link
Contributor

#18658 helps with this

@chiragramani
Copy link
Contributor Author

Confirming that #18658 helps big time.
On the latest run, the contribution of actuallyCompleteAction
Before: 14.8 seconds
Screenshot 2023-06-13 at 1 46 03 PM

After: 3s 177ms

Screenshot 2023-06-13 at 1 46 15 PM

@coeuvre
Copy link
Member

coeuvre commented Jun 14, 2023

Other than the linked PR for BLAKE3, we have implemented parallelized tree digest computation at HEAD by 368bf11. Would you mind check whether it improves actuallyCompleteAction?

@chiragramani
Copy link
Contributor Author

chiragramani commented Jun 14, 2023

Other than the linked PR for BLAKE3, we have implemented parallelized tree digest computation at HEAD by 368bf11. Would you mind check whether it improves actuallyCompleteAction?

I tested the initial benchmark shared in this issue with Bazel 6.2.1 which has the parallelized tree digest computation change but unfortunately, that didn't seem to improve. (comparing Bazel 6.1.1 which didn't have the parallelized tree digest computation change with Bazel 6.2.1 which had the change)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Remote-Exec Issues and PRs for the Execution (Remote) team type: bug
Projects
None yet
Development

No branches or pull requests

5 participants
@brentleyjones @coeuvre @chiragramani @Pavank1992 and others