Skip to content

Commit

Permalink
fix: stop copying targets, consume directly
Browse files Browse the repository at this point in the history
Since we have downloaded_file_path that points to the actual jar name
in our http_file invocations we can directly consume from those targets
instead of copying, this has an impact not just in IO as we reduce
copying, but also reduces the amount of targets bazel is aware as we
don't need an extra node, reducing the amount of RAM needed for
rules_jvm_external slightly.

In large maven repositories we use at Booking.com we saw a decrease in
the disk cache from 2.4GB to 300MB, which aligns with a repository_cache
of roughly 2.1GB. The time it takes to do a `bazel build @maven//...`
went from 120 seconds to 50 seconds on a i9-13900H using 75% of the cores.
  • Loading branch information
manuelnaranjo committed Mar 18, 2024
1 parent b29be75 commit 8abcc5f
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 25 deletions.
47 changes: 24 additions & 23 deletions private/dependency_tree_parser.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,13 @@ def _find_repository_url(artifact_url, repositories):
longest_match = repository
return longest_match

_ARTIFACT_JAR = """
alias(
name = "{artifact_path}",
actual = "{source}",
visibility = ["//visibility:public"],
)"""

def _generate_target(
repository_ctx,
jar_versionless_target_labels,
Expand All @@ -87,7 +94,7 @@ def _generate_target(
# (jvm|aar)_import(
#
packaging = artifact_path.split(".").pop()
if packaging == "jar":
if packaging == "jar" or artifact_path.endswith("//file"):
# Regular `java_import` invokes ijar on all JARs, causing some Scala and
# Kotlin compile interface JARs to be incorrect. We replace java_import
# with a simple jvm_import Starlark rule that skips ijar.
Expand Down Expand Up @@ -120,7 +127,7 @@ def _generate_target(
# srcjar = "https/repo1.maven.org/maven2/org/hamcrest/hamcrest-library/1.3/hamcrest-library-1.3-sources.jar",
#
is_dylib = False
if packaging == "jar":
if packaging == "jar" or artifact_path.endswith("//file"):
target_import_string.append("\tjars = [\"%s\"]," % artifact_path)
if srcjar_paths != None and target_label in srcjar_paths:
target_import_string.append("\tsrcjar = \"%s\"," % srcjar_paths[target_label])
Expand Down Expand Up @@ -320,18 +327,6 @@ genrule(
to_return.append("alias(\n\tname = \"%s\",\n\tactual = \"%s\",\n%s)" %
(versioned_target_alias_label, target_label, alias_visibility))

# 11. If using maven_install.json, use a genrule to copy the file from the http_file
# repository into this repository.
#
# genrule(
# name = "org_hamcrest_hamcrest_library_1_3_extension",
# srcs = ["@org_hamcrest_hamcrest_library_1_3//file"],
# outs = ["@maven//:v1/https/repo1.maven.org/maven2/org/hamcrest/hamcrest-library/1.3/hamcrest-library-1.3.jar"],
# cmd = "cp $< $@",
# )
if repository_ctx.attr.maven_install_json:
to_return.append(_genrule_copy_artifact_from_http_file(artifact, default_visibilities))

return to_return

# Generate BUILD file with jvm_import and aar_import for each artifact in
Expand All @@ -351,6 +346,8 @@ def _generate_imports(repository_ctx, dependencies, explicit_artifacts, neverlin
# seen_imports :: string -> bool
seen_imports = {}

added_aliases = {}

# A list of versionless target labels for jar artifacts. This is used for
# generating a compatibility layer for repositories. For example, if we generate
# @maven//:junit_junit, we also generate @junit_junit//jar as an alias to it.
Expand Down Expand Up @@ -378,8 +375,6 @@ def _generate_imports(repository_ctx, dependencies, explicit_artifacts, neverlin
seen_imports[artifact_path] = True
target_label = escape(strip_packaging_and_classifier_and_version(artifact["coordinates"]))
srcjar_paths[target_label] = artifact_path
if repository_ctx.attr.maven_install_json:
all_imports.append(_genrule_copy_artifact_from_http_file(artifact, default_visibilities))

# Iterate through the list of artifacts, and generate the target declaration strings.
for artifact in dependencies:
Expand Down Expand Up @@ -409,23 +404,18 @@ def _generate_imports(repository_ctx, dependencies, explicit_artifacts, neverlin
all_imports.append(
"alias(\n\tname = \"%s\",\n\tactual = \"%s\",\n\tvisibility = [\"//visibility:public\"],\n)" % (target_label, versioned_target_alias_label),
)
if repository_ctx.attr.maven_install_json:
all_imports.append(_genrule_copy_artifact_from_http_file(artifact, default_visibilities))
elif target_label in labels_to_override:
# Override target labels with the user provided mapping, instead of generating
# a jvm_import/aar_import based on information in dep_tree.
seen_imports[target_label] = True
all_imports.append(
"alias(\n\tname = \"%s\",\n\tactual = \"%s\",\n\tvisibility = [\"//visibility:public\"],)" % (target_label, labels_to_override.get(target_label)),
"alias(\n\tname = \"%s\",\n\tactual = \"%s\",\n\tvisibility = [\"//visibility:public\"],\n)" % (target_label, labels_to_override.get(target_label)),
)
if repository_ctx.attr.maven_install_json:
# Provide the downloaded artifact as a file target.
all_imports.append(_genrule_copy_artifact_from_http_file(artifact, default_visibilities))
raw_artifact = dict(artifact)
raw_artifact["coordinates"] = "original_" + artifact["coordinates"]
raw_artifact["maven_coordinates"] = artifact["coordinates"]
raw_artifact["out"] = "original_" + artifact["file"]

raw_artifact["file"] = "@%s//file" % escape(artifact["coordinates"])
all_imports.extend(_generate_target(
repository_ctx,
jar_versionless_target_labels,
Expand All @@ -439,7 +429,18 @@ def _generate_imports(repository_ctx, dependencies, explicit_artifacts, neverlin
raw_artifact,
))


elif artifact_path != None:
if artifact["file"] not in added_aliases:
added_aliases[artifact["file"]] = True
repo = escape(artifact["coordinates"])
all_imports.append(
_ARTIFACT_JAR.format(
artifact_path = artifact["file"],
source = "@%s//file" % repo
)
)

all_imports.extend(_generate_target(
repository_ctx,
jar_versionless_target_labels,
Expand Down
4 changes: 2 additions & 2 deletions private/rules/jvm_import.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def _jvm_import_impl(ctx):

injar = ctx.files.jars[0]
if ctx.attr._stamp_manifest[StampManifestProvider].stamp_enabled:
outjar = ctx.actions.declare_file("processed_" + injar.basename, sibling = injar)
outjar = ctx.actions.declare_file("processed_" + injar.basename)
args = ctx.actions.args()
args.add_all(["--source", injar, "--output", outjar])
args.add_all(["--manifest-entry", "Target-Label:{target_label}".format(target_label = label)])
Expand All @@ -40,7 +40,7 @@ def _jvm_import_impl(ctx):
else:
outjar = injar

compilejar = ctx.actions.declare_file("header_" + injar.basename, sibling = injar)
compilejar = ctx.actions.declare_file("header_" + injar.basename)
args = ctx.actions.args()
args.add_all(["--source", outjar, "--output", compilejar])

Expand Down

0 comments on commit 8abcc5f

Please sign in to comment.