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

Fix symlink file creation overhead #17478

Closed
wants to merge 2 commits into from

Conversation

coeuvre
Copy link
Member

@coeuvre coeuvre commented Feb 13, 2023

The cost of symlink action scales with the size of input because Bazel re-calculates the digest of the output by following the symlink in actuallyCompleteAction (#14125). However, the re-calculation is redundant because the digest was already computed by Bazel when checking the outputs of the generating action. Bazel should be smart enough to reuse the result.

There is a global cache in Bazel for digest computation. Symlink action didn't make use of the cache because it uses the path of symlink as key to look up the cache. This PR changes to use the path of input file (i.e. target path) to query the cache to avoid recalculation.

@coeuvre coeuvre changed the title Fix symlink creation overhead Fix symlink file creation overhead Feb 13, 2023
@coeuvre coeuvre marked this pull request as ready for review February 13, 2023 15:40
@coeuvre coeuvre requested a review from tjgq February 13, 2023 15:40
@fmeum
Copy link
Collaborator

fmeum commented Feb 13, 2023

@bazel-io flag

@bazel-io bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Feb 13, 2023
@sgowroji sgowroji added the team-Remote-Exec Issues and PRs for the Execution (Remote) team label Feb 13, 2023
@coeuvre
Copy link
Member Author

coeuvre commented Feb 14, 2023

@bazel-io fork 6.1.0

@coeuvre coeuvre deleted the fix-symlink-check branch February 14, 2023 15:45
@bazel-io bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Feb 14, 2023
hvadehra pushed a commit that referenced this pull request Feb 14, 2023
The cost of symlink action scales with the size of input because Bazel re-calculates the digest of the output by following the symlink in `actuallyCompleteAction` (#14125). However, the re-calculation is redundant because the digest was already computed by Bazel when checking the outputs of the generating action. Bazel should be smart enough to reuse the result.

There is a global cache in Bazel for digest computation. Symlink action didn't make use of the cache because it uses the path of symlink as key to look up the cache. This PR changes to use the path of input file (i.e. target path) to query the cache to avoid recalculation.

For a large target (700MB), the time for symlink action is reduced from 2000ms to 1ms.

Closes #17478.

PiperOrigin-RevId: 509524641
Change-Id: Id3c9dc07d68758770c092f6307e2433dad40ba10
keertk added a commit that referenced this pull request Feb 20, 2023
* Fix symlink file creation overhead

The cost of symlink action scales with the size of input because Bazel re-calculates the digest of the output by following the symlink in `actuallyCompleteAction` (#14125). However, the re-calculation is redundant because the digest was already computed by Bazel when checking the outputs of the generating action. Bazel should be smart enough to reuse the result.

There is a global cache in Bazel for digest computation. Symlink action didn't make use of the cache because it uses the path of symlink as key to look up the cache. This PR changes to use the path of input file (i.e. target path) to query the cache to avoid recalculation.

For a large target (700MB), the time for symlink action is reduced from 2000ms to 1ms.

Closes #17478.

PiperOrigin-RevId: 509524641
Change-Id: Id3c9dc07d68758770c092f6307e2433dad40ba10

* Update ActionMetadataHandler.java

* Create OutputPermissions.java

---------

Co-authored-by: Chi Wang <chiwang@google.com>
Co-authored-by: keertk <110264242+keertk@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Remote-Exec Issues and PRs for the Execution (Remote) team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants