Skip to content

Commit

Permalink
Add less aggressive performance fixes back (#18)
Browse files Browse the repository at this point in the history
* Skip building unnecessary jars (cherry-picked commit 1c6073a)
* Skip building resolve files if the target is imported as a source (cherry-picked commit 86e646a)
* Increment the version

Co-authored-by: Uri Baghin <[email protected]>
  • Loading branch information
Serge Belov and uri-canva authored Aug 10, 2022
1 parent ee8b0dd commit 5e59a3c
Show file tree
Hide file tree
Showing 9 changed files with 42 additions and 29 deletions.
43 changes: 39 additions & 4 deletions aspect/intellij_info_impl.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,10 @@ def jars_from_output(output):
"""Collect jars for intellij-resolve-files from Java output."""
if output == None:
return []
source_jars = get_source_jars(output)
return [
jar
for jar in ([output.class_jar, output.ijar] + get_source_jars(output))
for jar in ([output.ijar if len(source_jars) > 0 and output.ijar else output.class_jar] + source_jars)
if jar != None and not jar.is_source
]

Expand Down Expand Up @@ -570,6 +571,40 @@ def _collect_generated_files(java):
return [(java.annotation_processing.class_jar, java.annotation_processing.source_jar)]
return []

def import_as_java_source(kind, sources):
"""Whether to import the given target as source or as a library.
https://github.com/bazelbuild/intellij/blob/28548e388093f986bb8fbd666917c3b42c434f6d/java/src/com/google/idea/blaze/java/sync/importer/JavaSourceFilter.java#L113"""
# Assume we're interested in all targets, ignore `targets` in the project view
return is_java_source_target(kind, sources) or is_java_proto_target(kind)

def is_java_source_target(kind, sources):
return can_import_as_java_source(kind) and has_non_generated_sources(sources)

def can_import_as_java_source(kind):
return kind not in [
"aar_import",
"java_import",
"java_wrap_cc",
"kotlin_stdlib",
"kt_jvm_import",
"scala_import",
]

def has_non_generated_sources(sources):
for source in sources:
if source.is_source:
return True
return False

def is_java_proto_target(kind):
return kind in [
"java_lite_proto_library",
"java_mutable_proto_library",
"java_proto_library",
"proto_library",
]

def collect_java_info(target, ctx, semantics, ide_info, ide_info_file, output_groups):
"""Updates Java-specific output groups, returns false if not a Java target."""
java = get_java_provider(target)
Expand All @@ -591,7 +626,7 @@ def collect_java_info(target, ctx, semantics, ide_info, ide_info_file, output_gr
jars = [library_artifact(output) for output in java_outputs]
class_jars = [output.class_jar for output in java_outputs if output and output.class_jar]
output_jars = [jar for output in java_outputs for jar in jars_from_output(output)]
resolve_files = output_jars
resolve_files = [] if import_as_java_source(ctx.rule.kind, sources) else output_jars
compile_files = class_jars

gen_jars = []
Expand Down Expand Up @@ -714,12 +749,12 @@ def _build_filtered_gen_jar(ctx, target, java_outputs, gen_java_sources, srcjars
for jar in java_outputs:
if jar.ijar:
jar_artifacts.append(jar.ijar)
elif jar.class_jar:
jar_artifacts.append(jar.class_jar)
if hasattr(jar, "source_jars") and jar.source_jars:
source_jar_artifacts.extend(jar.source_jars)
elif hasattr(jar, "source_jar") and jar.source_jar:
source_jar_artifacts.append(jar.source_jar)
if len(source_jar_artifacts) == 0 or len(jar_artifacts) == 0:
jar_artifacts.extend([jar.class_jar for jar in java_outputs if jar.class_jar])

filtered_jar = ctx.actions.declare_file(target.label.name + "-filtered-gen.jar")
filtered_source_jar = ctx.actions.declare_file(target.label.name + "-filtered-gen-src.jar")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ public void testNoIde() throws Exception {
testRelative("foo.java-manifest"), testRelative(intellijInfoFileName("foo")));
assertThat(getOutputGroupFiles(testFixture, "intellij-resolve-java"))
.containsExactly(
testRelative("libfoo.jar"),
testRelative("libfoo-hjar.jar"),
testRelative("libfoo-src.jar"),
testRelative("libfoo.jdeps"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,10 @@ public void testJavaLibraryWithTransitiveDependencies() throws Exception {
testRelative(intellijInfoFileName("transitive_dep")));
assertThat(getOutputGroupFiles(testFixture, "intellij-resolve-java"))
.containsAllOf(
testRelative("libfoo.jar"),
testRelative("libfoo-hjar.jar"),
testRelative("libfoo-src.jar"),
testRelative("libsingle_dep.jar"),
testRelative("libsingle_dep-hjar.jar"),
testRelative("libsingle_dep-src.jar"),
testRelative("libtransitive_dep.jar"),
testRelative("libtransitive_dep-hjar.jar"),
testRelative("libtransitive_dep-src.jar"));
assertThat(getOutputGroupFiles(testFixture, "intellij-compile-java"))
Expand Down Expand Up @@ -103,13 +100,10 @@ public void testJavaLibraryWithExports() throws Exception {
testRelative(intellijInfoFileName("export_consumer")));
assertThat(getOutputGroupFiles(testFixture, "intellij-resolve-java"))
.containsAllOf(
testRelative("libfoo.jar"),
testRelative("libfoo-hjar.jar"),
testRelative("libfoo-src.jar"),
testRelative("libfoo_exporter.jar"),
testRelative("libfoo_exporter-hjar.jar"),
testRelative("libfoo_exporter-src.jar"),
testRelative("libexport_consumer.jar"),
testRelative("libexport_consumer-hjar.jar"),
testRelative("libexport_consumer-src.jar"));
assertThat(getOutputGroupFiles(testFixture, "intellij-compile-java"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,8 @@ public void testJavaBinary() throws Exception {
testRelative(intellijInfoFileName("foo")));
assertThat(getOutputGroupFiles(testFixture, "intellij-resolve-java"))
.containsAllOf(
testRelative("libfoolib.jar"),
testRelative("libfoolib-hjar.jar"),
testRelative("libfoolib-src.jar"),
testRelative("foo.jar"),
testRelative("foo-src.jar"));
assertThat(getOutputGroupFiles(testFixture, "intellij-compile-java"))
.containsExactly(testRelative("libfoolib.jar"), testRelative("foo.jar"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,40 +123,33 @@ public void testJavaLibrary() throws Exception {
assertThat(getOutputGroupFiles(testFixture, "intellij-resolve-java"))
.containsExactly(
// foo
testRelative("libfoo.jar"),
testRelative("libfoo-hjar.jar"),
testRelative("libfoo-src.jar"),
testRelative("libfoo.jdeps"),
// direct
testRelative("libdirect.jar"),
testRelative("libdirect-hjar.jar"),
testRelative("libdirect-src.jar"),
testRelative("libdirect.jdeps"),
// indirect
testRelative("libindirect.jar"),
testRelative("libindirect-hjar.jar"),
testRelative("libindirect-src.jar"),
testRelative("libindirect.jdeps"),
// distant
testRelative("libdistant.jar"),
testRelative("libdistant-hjar.jar"),
testRelative("libdistant-src.jar"),
testRelative("libdistant.jdeps"));
assertThat(getOutputGroupFiles(testFixture, "intellij-resolve-java-outputs"))
.containsExactly(
testRelative("libfoo.jar"),
testRelative("libfoo-hjar.jar"),
testRelative("libfoo-src.jar"),
testRelative("libfoo.jdeps"));
assertThat(getOutputGroupFiles(testFixture, "intellij-resolve-java-direct-deps"))
.containsExactly(
// foo
testRelative("libfoo.jar"),
testRelative("libfoo-hjar.jar"),
testRelative("libfoo-src.jar"),
testRelative("libfoo.jdeps"),
// direct
testRelative("libdirect.jar"),
testRelative("libdirect-hjar.jar"),
testRelative("libdirect-src.jar"),
testRelative("libdirect.jdeps"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,33 +108,27 @@ public void testJavaProtoLibrary() throws Exception {
assertThat(getOutputGroupFiles(testFixture, "intellij-resolve-java"))
.containsExactly(
// lib
testRelative("liblib.jar"),
testRelative("liblib-hjar.jar"),
testRelative("liblib-src.jar"),
testRelative("liblib.jdeps"),
// bar_proto
testRelative("libbar_proto-speed.jar"),
testRelative("libbar_proto-speed-hjar.jar"),
testRelative("bar_proto-speed-src.jar"),
// foo_proto
testRelative("libfoo_proto-speed.jar"),
testRelative("libfoo_proto-speed-hjar.jar"),
testRelative("foo_proto-speed-src.jar"));
assertThat(getOutputGroupFiles(testFixture, "intellij-resolve-java-outputs"))
.containsExactly(
testRelative("liblib.jar"),
testRelative("liblib-hjar.jar"),
testRelative("liblib-src.jar"),
testRelative("liblib.jdeps"));
assertThat(getOutputGroupFiles(testFixture, "intellij-resolve-java-direct-deps"))
.containsAllOf(
// lib
testRelative("liblib.jar"),
testRelative("liblib-hjar.jar"),
testRelative("liblib-src.jar"),
testRelative("liblib.jdeps"),
// bar_proto
testRelative("libbar_proto-speed.jar"),
testRelative("libbar_proto-speed-hjar.jar"),
testRelative("bar_proto-speed-src.jar"),
// foo_proto (only hjar)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public void testScalaBinary() throws Exception {
testRelative(intellijInfoFileName("foolib")),
testRelative(intellijInfoFileName("foo")));
assertThat(getOutputGroupFiles(testFixture, "intellij-resolve-java"))
.containsAllOf(testRelative("foolib.jar"), testRelative("foo.jar"));
.containsAllOf(testRelative("foolib-ijar.jar"), testRelative("foo.jar"));
assertThat(getOutputGroupFiles(testFixture, "intellij-compile-java"))
.containsAllOf(testRelative("foolib.jar"), testRelative("foo.jar"));
assertThat(getOutputGroupFiles(testFixture, "intellij-info-generic")).isEmpty();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public void testScalaLibrary() throws Exception {
// Also contains ijars for scala-library.
// Also contains jars + srcjars for liblibrary.
assertThat(getOutputGroupFiles(testFixture, "intellij-resolve-java"))
.contains(testRelative("simple.jar"));
.contains(testRelative("simple-ijar.jar"));

assertThat(getOutputGroupFiles(testFixture, "intellij-info-java"))
.contains(testRelative(intellijInfoFileName("simple")));
Expand Down
2 changes: 1 addition & 1 deletion version.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@
# default version to 9999 so that a dev plugin built from Piper HEAD will override any production
# plugin (because IntelliJ will choose the highest version when it sees two conflicting plugins, so
# 9999 > 2017.06.05.0.1).
VERSION = "2032.07.26.0.0"
VERSION = "2032.07.26.1.0"

0 comments on commit 5e59a3c

Please sign in to comment.