From e16de1b994c7ffe528f8445881eee0d573c7e3e9 Mon Sep 17 00:00:00 2001 From: Googler Date: Fri, 17 Nov 2023 08:40:01 -0800 Subject: [PATCH] Delete experimental `java_library` sharded compilation support PiperOrigin-RevId: 583398378 Change-Id: Ia01c945574106d95097275c053f6f4d89f1eb042 --- .../starlark/builtins_bzl/bazel/exports.bzl | 3 +- .../java_common_internal_for_builtins.bzl | 105 ++++++-------- .../builtins_bzl/common/java/java_library.bzl | 36 ++--- .../common/java/sharded_javac.bzl | 129 ------------------ .../rules/java/JavaConfiguredTargetsTest.java | 73 ---------- 5 files changed, 52 insertions(+), 294 deletions(-) delete mode 100644 src/main/starlark/builtins_bzl/common/java/sharded_javac.bzl diff --git a/src/main/starlark/builtins_bzl/bazel/exports.bzl b/src/main/starlark/builtins_bzl/bazel/exports.bzl index 63d6b5b6e9db38..668d6f3f3565ef 100644 --- a/src/main/starlark/builtins_bzl/bazel/exports.bzl +++ b/src/main/starlark/builtins_bzl/bazel/exports.bzl @@ -16,7 +16,7 @@ load("@_builtins//:common/cc/cc_proto_library.bzl", "cc_proto_aspect", "cc_proto_library") load("@_builtins//:common/java/java_import.bzl", "java_import") -load("@_builtins//:common/java/java_library.bzl", "JAVA_LIBRARY_ATTRS", "bazel_java_library_rule", "java_library", "make_sharded_java_library") +load("@_builtins//:common/java/java_library.bzl", "JAVA_LIBRARY_ATTRS", "bazel_java_library_rule", "java_library") load("@_builtins//:common/java/java_plugin.bzl", "java_plugin") load("@_builtins//:common/java/proto/java_proto_library.bzl", "java_proto_library") load("@_builtins//:common/proto/proto_library.bzl", "proto_library") @@ -33,7 +33,6 @@ exported_toplevels = { "experimental_java_library_export_do_not_use": struct( bazel_java_library_rule = bazel_java_library_rule, JAVA_LIBRARY_ATTRS = JAVA_LIBRARY_ATTRS, - sharded_java_library = make_sharded_java_library, ), "cc_proto_aspect": cc_proto_aspect, "py_internal": py_internal, diff --git a/src/main/starlark/builtins_bzl/common/java/java_common_internal_for_builtins.bzl b/src/main/starlark/builtins_bzl/common/java/java_common_internal_for_builtins.bzl index 6115d207d23f8d..dfce461cbfb05d 100644 --- a/src/main/starlark/builtins_bzl/common/java/java_common_internal_for_builtins.bzl +++ b/src/main/starlark/builtins_bzl/common/java/java_common_internal_for_builtins.bzl @@ -24,7 +24,6 @@ load( ) load(":common/java/java_semantics.bzl", "semantics") load(":common/java/java_toolchain.bzl", "JavaToolchainInfo") -load(":common/java/sharded_javac.bzl", "experimental_sharded_javac", "use_sharded_javac") load(":common/paths.bzl", "paths") _java_common_internal = _builtins.internal.java_common_internal_do_not_use @@ -223,71 +222,45 @@ def compile( compile_jar = output compile_deps_proto = None - if use_sharded_javac(ctx): - if compile_jar == output or not compile_jar: - fail("sharding requested without hjar/ijar compilation") - generated_source_jar = None - generated_class_jar = None - deps_proto = None - native_headers_jar = None - manifest_proto = None - experimental_sharded_javac( - ctx, - java_toolchain, - output, - compile_jar, - plugin_info, - compilation_classpath, - direct_jars, - bootclasspath, - compile_time_java_deps, - all_javac_opts, - strict_deps, - source_files, - source_jars, - resources, - resource_jars, - ) - else: - native_headers_jar = helper.derive_output_file(ctx, output, name_suffix = "-native-header") - manifest_proto = helper.derive_output_file(ctx, output, extension_suffix = "_manifest_proto") - deps_proto = None - if ctx.fragments.java.generate_java_deps() and has_sources: - deps_proto = helper.derive_output_file(ctx, output, extension = "jdeps") - generated_class_jar = None - generated_source_jar = None - if uses_annotation_processing: - generated_class_jar = helper.derive_output_file(ctx, output, name_suffix = "-gen") - generated_source_jar = helper.derive_output_file(ctx, output, name_suffix = "-gensrc") - _java_common_internal.create_compilation_action( - ctx, - java_toolchain, - output, - manifest_proto, - plugin_info, - compilation_classpath, - direct_jars, - bootclasspath, - compile_time_java_deps, - all_javac_opts, - strict_deps, - ctx.label, - deps_proto, - generated_class_jar, - generated_source_jar, - native_headers_jar, - depset(source_files), - source_jars, - resources, - depset(resource_jars), - classpath_resources, - sourcepath, - injecting_rule_kind, - enable_jspecify, - enable_direct_classpath, - annotation_processor_additional_inputs, - annotation_processor_additional_outputs, - ) + native_headers_jar = helper.derive_output_file(ctx, output, name_suffix = "-native-header") + manifest_proto = helper.derive_output_file(ctx, output, extension_suffix = "_manifest_proto") + deps_proto = None + if ctx.fragments.java.generate_java_deps() and has_sources: + deps_proto = helper.derive_output_file(ctx, output, extension = "jdeps") + generated_class_jar = None + generated_source_jar = None + if uses_annotation_processing: + generated_class_jar = helper.derive_output_file(ctx, output, name_suffix = "-gen") + generated_source_jar = helper.derive_output_file(ctx, output, name_suffix = "-gensrc") + _java_common_internal.create_compilation_action( + ctx, + java_toolchain, + output, + manifest_proto, + plugin_info, + compilation_classpath, + direct_jars, + bootclasspath, + compile_time_java_deps, + all_javac_opts, + strict_deps, + ctx.label, + deps_proto, + generated_class_jar, + generated_source_jar, + native_headers_jar, + depset(source_files), + source_jars, + resources, + depset(resource_jars), + classpath_resources, + sourcepath, + injecting_rule_kind, + enable_jspecify, + enable_direct_classpath, + annotation_processor_additional_inputs, + annotation_processor_additional_outputs, + ) create_output_source_jar = len(source_files) > 0 or source_jars != [output_source_jar] if not output_source_jar: diff --git a/src/main/starlark/builtins_bzl/common/java/java_library.bzl b/src/main/starlark/builtins_bzl/common/java/java_library.bzl index e803c9ce7cf337..48bef5af8c88f9 100644 --- a/src/main/starlark/builtins_bzl/common/java/java_library.bzl +++ b/src/main/starlark/builtins_bzl/common/java/java_library.bzl @@ -167,27 +167,15 @@ JAVA_LIBRARY_ATTRS = merge_attrs( }, ) -def _make_java_library_rule(extra_attrs = {}): - return rule( - _proxy, - attrs = merge_attrs( - JAVA_LIBRARY_ATTRS, - extra_attrs, - ), - provides = [JavaInfo], - outputs = { - "classjar": "lib%{name}.jar", - "sourcejar": "lib%{name}-src.jar", - }, - fragments = ["java", "cpp"], - toolchains = [semantics.JAVA_TOOLCHAIN], - subrules = [android_lint_subrule], - ) - -java_library = _make_java_library_rule() - -# for experimental_java_library_export_do_not_use -def make_sharded_java_library(default_shard_size): - return _make_java_library_rule({ - "experimental_javac_shard_size": attr.int(default = default_shard_size), - }) +java_library = rule( + _proxy, + attrs = JAVA_LIBRARY_ATTRS, + provides = [JavaInfo], + outputs = { + "classjar": "lib%{name}.jar", + "sourcejar": "lib%{name}-src.jar", + }, + fragments = ["java", "cpp"], + toolchains = [semantics.JAVA_TOOLCHAIN], + subrules = [android_lint_subrule], +) diff --git a/src/main/starlark/builtins_bzl/common/java/sharded_javac.bzl b/src/main/starlark/builtins_bzl/common/java/sharded_javac.bzl deleted file mode 100644 index 6306aecb7fd295..00000000000000 --- a/src/main/starlark/builtins_bzl/common/java/sharded_javac.bzl +++ /dev/null @@ -1,129 +0,0 @@ -# Copyright 2023 The Bazel Authors. All rights reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -""" Highly experimental/unstable implementation of sharded compilation """ - -load(":common/java/java_helper.bzl", "helper") - -_java_common_internal = _builtins.internal.java_common_internal_do_not_use - -def use_sharded_javac(ctx): - """Whether this target should use sharded javac compilation - - Args: - ctx: the rule context - - Returns: - (bool) - """ - if not hasattr(ctx.attr, "experimental_javac_shard_size"): - return False - if ctx.attr.experimental_javac_shard_size <= 0: - fail("invalid value for javac shard size, expected a value > 0") - return True - -def experimental_sharded_javac( - ctx, - java_toolchain, - output, - compile_jar, - plugin_info, - compilation_classpath, - direct_jars, - bootclasspath, - compile_time_java_deps, - all_javac_opts, - strict_deps, - source_files, - source_jars, - resources, - resource_jars): - """ Performs multiple javac compilations and merges the results with singlejar - - Size of each shard is determined by ctx.attr.experimental_javac_shard_size, - and there is one action for all resources, source_jars and resource_jars. - - Args: - ctx: the rule context - java_toolchain: the java toolchain - output: (File) the final class jar to produce - compile_jar: (File) the output of the header jar compilation for this target - plugin_info: (JavaPluginInfo) information about plugins - compilation_classpath: (depset[File]) the transitive jars required for compilation - direct_jars: (depset[File]) the direct jars required for compilation - bootclasspath: (BootClassPathInfo) the bootclasspath to use - compile_time_java_deps: (depset[File]) - all_javac_opts: (depset[str]) options to pass to javabuilder - strict_deps: (str) the strict deps mode - source_files: [File] list of all the sources files to compile - source_jars: [File] the list of jars containing sources - resources: [File] the list of resource files - resource_jars: [File] the list of jars containing resources - """ - shard_direct_jars = depset(direct = [compile_jar], transitive = [direct_jars]) - shard_compilation_classpath = depset(direct = [compile_jar], transitive = [compilation_classpath]) - - sharded_outputs = [] - shard_size = ctx.attr.experimental_javac_shard_size - shard_count = (len(source_files) + shard_size - 1) // shard_size - for shard_idx in range(shard_count): - start = shard_idx * shard_size - sources_for_shard = source_files[start:start + shard_size] - shard_output = helper.derive_output_file(ctx, output, name_suffix = "_shard_%s" % shard_idx) - sharded_outputs.append(shard_output) - _java_common_internal.create_compilation_action( - ctx, - java_toolchain, - shard_output, - helper.derive_output_file(ctx, shard_output, extension_suffix = "_manifest_proto"), # manifest_proto - plugin_info, - shard_compilation_classpath, - shard_direct_jars, - bootclasspath, - compile_time_java_deps, - all_javac_opts, - strict_deps, - ctx.label, - sources = depset(sources_for_shard), - ) - if resources or resource_jars or source_jars: - shard_output = helper.derive_output_file(ctx, output, name_suffix = "_shard_resources") - sharded_outputs.append(shard_output) - _java_common_internal.create_compilation_action( - ctx, - java_toolchain, - shard_output, - helper.derive_output_file(ctx, shard_output, extension_suffix = "_manifest_proto"), # manifest_proto - plugin_info, - shard_compilation_classpath, - shard_direct_jars, - bootclasspath, - compile_time_java_deps, - all_javac_opts, - strict_deps, - ctx.label, - source_jars = source_jars, - resources = resources, - resource_jars = depset(resource_jars), - ) - helper.create_single_jar( - ctx.actions, - toolchain = java_toolchain, - output = output, - sources = depset(sharded_outputs), - progress_message = "Merging shards into %{output}", - mnemonic = "JavaSingleJar", - build_target = ctx.label, - output_creator = "bazel", - ) diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/java/JavaConfiguredTargetsTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/java/JavaConfiguredTargetsTest.java index ae3d8581c0b24d..fadb56b8c2a797 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/rules/java/JavaConfiguredTargetsTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/java/JavaConfiguredTargetsTest.java @@ -15,22 +15,14 @@ package com.google.devtools.build.lib.bazel.rules.java; import static com.google.common.collect.ImmutableList.toImmutableList; -import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.collect.MoreCollectors.onlyElement; import static com.google.common.truth.Truth.assertThat; import static com.google.devtools.build.lib.testutil.TestConstants.TOOLS_REPOSITORY; import com.google.common.base.Joiner; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; -import com.google.common.io.MoreFiles; -import com.google.devtools.build.lib.actions.Action; -import com.google.devtools.build.lib.actions.ActionAnalysisMetadata; -import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.actions.TemplateExpansionAction; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; -import com.google.devtools.build.lib.rules.java.JavaCompileAction; import com.google.devtools.build.lib.util.OS; import java.util.Arrays; import java.util.Objects; @@ -101,69 +93,4 @@ public void javaTestSetsSecurityManagerPropertyOnVersion17() throws Exception { assertThat(jvmFlags).contains("-Djava.security.manager=allow"); } } - - @Test - public void experimentalShardedJavaLibrary_succeeds() throws Exception { - setBuildLanguageOptions("--experimental_java_library_export"); - scratch.file( - "foo/rule.bzl", - // - "java_library = experimental_java_library_export_do_not_use.sharded_java_library(", - " default_shard_size = 10", - ")"); - scratch.file( - "foo/BUILD", - "load(':rule.bzl', 'java_library')", - "", - "java_library(", - " name = 'lib1',", - " srcs = ['1.java', '2.java', '3.java'],", - " experimental_javac_shard_size = 1,", - ")", - "java_library(", - " name = 'lib2',", - " srcs = ['1.java', '2.java', '3.java'],", - " experimental_javac_shard_size = 2,", - ")"); - - ImmutableList compileActionsWithShardSize1 = - getActions("//foo:lib1", JavaCompileAction.class); - ImmutableList compileActionsWithShardSize2 = - getActions("//foo:lib2", JavaCompileAction.class); - - assertThat(compileActionsWithShardSize1).hasSize(3); - assertThat(compileActionsWithShardSize2).hasSize(2); - } - - // regression test for b/297356812#comment31 - @Test - public void experimentalShardedJavaLibrary_allOutputsHaveUniqueNames() throws Exception { - setBuildLanguageOptions("--experimental_java_library_export"); - scratch.file( - "foo/rule.bzl", - // - "java_library = experimental_java_library_export_do_not_use.sharded_java_library(", - " default_shard_size = 1", - ")"); - scratch.file( - "foo/BUILD", - "load(':rule.bzl', 'java_library')", - "", - "java_library(", - " name = 'lib',", - " srcs = ['1.java', '2.java', '3.java'],", - ")"); - - ImmutableList outputs = - getActions("//foo:lib", JavaCompileAction.class).stream() - .map(ActionAnalysisMetadata::getPrimaryOutput) - .collect(toImmutableList()); - ImmutableSet uniqueFilenamesWithoutExtension = - outputs.stream() - .map(file -> MoreFiles.getNameWithoutExtension(file.getPath().getPathFile().toPath())) - .collect(toImmutableSet()); - - assertThat(outputs).hasSize(3); - assertThat(uniqueFilenamesWithoutExtension).hasSize(3); - } }