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

Improve repinning performance by not recomputing cache path for every artifact #1108

Merged
merged 2 commits into from
Apr 23, 2024

Conversation

BalestraPatrick
Copy link
Contributor

We recently investigated the performance of rules_jvm_external because running REPIN=1 bazel run @unpinned_maven//:pin was taking about 140s in our project. With this patch, it now takes about 20s.

The bottleneck seemed to be get_coursier_cache_or_default which was being invoked for every single artifact in a loop. Our repos have more ~1k artifacts, so this was a substantial amount of work.

Copy link
Collaborator

@jin jin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

coursier.bzl Outdated Show resolved Hide resolved
@jin jin merged commit b02fa09 into bazel-contrib:master Apr 23, 2024
8 checks passed
@BalestraPatrick BalestraPatrick deleted the improve-performance branch April 23, 2024 14:22
@mattnworb
Copy link
Contributor

@jin any clue if there is a new release of rules_jvm_external in the plans for any time soon? We'd love to not have to add this as a patch on top of RJE in our workspace (I work with @BalestraPatrick )

@jin
Copy link
Collaborator

jin commented Apr 23, 2024

Yeah we can cut 6.1 pretty soon. @shs96c

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.

3 participants