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

Optimize classpath pre-processing in java_stub_template.txt #19481

Closed

Conversation

rsalvador
Copy link
Contributor

@rsalvador rsalvador commented Sep 11, 2023

The classpath pre-processing in this java_stub_template.txt loop:

is slow for long classpaths. For example for classpaths with ~250,000 and ~700,000 entries the loop takes 28 and 50 seconds, respectively, on an intel MacBook. This change reduce the times to 1 second or less.

Fixes #19480

@github-actions github-actions bot added the awaiting-review PR is awaiting review from an assigned reviewer label Sep 11, 2023
@fmeum
Copy link
Collaborator

fmeum commented Sep 11, 2023

@mai93 As the original author of d0ee889, would you be available to review this?

@sgowroji sgowroji added the team-Performance Issues for Performance teams label Sep 11, 2023
@sgowroji
Copy link
Member

Hi @rsalvador, Could you please take a look at the failing checks?

@sgowroji sgowroji added awaiting-user-response Awaiting a response from the author and removed awaiting-review PR is awaiting review from an assigned reviewer labels Sep 11, 2023
@rsalvador
Copy link
Contributor Author

rsalvador commented Sep 11, 2023

Hi @sgowroji, seems to be an infra (No space left on device) error: https://buildkite.com/bazel/bazel-bazel-github-presubmit/builds/17414#018a8379-ac66-45cd-b1d3-5319ee26895b/663-3842

(11:23:45) ERROR: /Users/buildkite/builds/bk-imacpro-6/bazel/bazel-bazel-github-presubmit/src/test/shell/bazel/BUILD:655:8: Testing //src/test/shell/bazel:allowlist_test (shard 1 of 3) failed: I/O exception during sandboxed execution: /private/var/tmp/_bazel_buildkite/c960f1e32dbf1661ca10ecfcb6915aad/sandbox/darwin-sandbox/6590/execroot/_main/bazel-out (No space left on device)

I see other PRs that trigger a darwin build also failing with the same error.

@sgowroji sgowroji added awaiting-review PR is awaiting review from an assigned reviewer and removed awaiting-user-response Awaiting a response from the author labels Sep 11, 2023
@mai93 mai93 added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Sep 11, 2023
@rsalvador
Copy link
Contributor Author

Could this fix be available in the 6.4.0 release? Do I need to do anything for that?

@fmeum
Copy link
Collaborator

fmeum commented Sep 11, 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 Sep 11, 2023
@iancha1992
Copy link
Member

@bazel-io fork 6.4.0

@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 Sep 11, 2023
@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Sep 11, 2023
bazel-io pushed a commit to bazel-io/bazel that referenced this pull request Sep 11, 2023
The classpath pre-processing in this `java_stub_template.txt` loop: https://github.com/bazelbuild/bazel/blob/fcfcb929366dd3faac9643302b19c88bcf871ec6/src/main/java/com/google/devtools/build/lib/bazel/rules/java/java_stub_template.txt#L309 is slow for long classpaths. For example for classpaths with ~250,000 and ~700,000 entries the loop takes 28 and 50 seconds, respectively, on an intel MacBook. This change reduce the times to 1 second or less.

Fixes bazelbuild#19480

Closes bazelbuild#19481.

PiperOrigin-RevId: 564491123
Change-Id: Id4be898c3f800d5390dd8bf997535a5e71a76ba3
iancha1992 pushed a commit that referenced this pull request Sep 12, 2023
…19491)

The classpath pre-processing in this `java_stub_template.txt` loop:
https://github.com/bazelbuild/bazel/blob/fcfcb929366dd3faac9643302b19c88bcf871ec6/src/main/java/com/google/devtools/build/lib/bazel/rules/java/java_stub_template.txt#L309
is slow for long classpaths. For example for classpaths with ~250,000
and ~700,000 entries the loop takes 28 and 50 seconds, respectively, on
an intel MacBook. This change reduce the times to 1 second or less.

Fixes #19480

Closes #19481.

Commit
4e8f0bd

PiperOrigin-RevId: 564491123
Change-Id: Id4be898c3f800d5390dd8bf997535a5e71a76ba3

Co-authored-by: Roman Salvador <[email protected]>
SalmaSamy pushed a commit that referenced this pull request Sep 13, 2023
…19491)

The classpath pre-processing in this `java_stub_template.txt` loop:
https://github.com/bazelbuild/bazel/blob/fcfcb929366dd3faac9643302b19c88bcf871ec6/src/main/java/com/google/devtools/build/lib/bazel/rules/java/java_stub_template.txt#L309
is slow for long classpaths. For example for classpaths with ~250,000
and ~700,000 entries the loop takes 28 and 50 seconds, respectively, on
an intel MacBook. This change reduce the times to 1 second or less.

Fixes #19480

Closes #19481.

Commit
4e8f0bd

PiperOrigin-RevId: 564491123
Change-Id: Id4be898c3f800d5390dd8bf997535a5e71a76ba3

Co-authored-by: Roman Salvador <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Performance Issues for Performance teams
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Long runtime classpaths slow down unit test execution
6 participants