Skip to content

Commit dee33f4

Browse files
authored
java_stub_template to use argument file instead of classpath jar (#1410)
* java_stub_template to use argument file instead of classpath jar (shaves 1s-1.5s, works java>8)
1 parent 73f5d1a commit dee33f4

File tree

6 files changed

+67
-4
lines changed

6 files changed

+67
-4
lines changed

java_stub_template/file/file.txt

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,14 @@ ARGS=(
289289
"${ARGS[@]}")
290290

291291

292+
function run_test_with_large_classpath() {
293+
if [ "%test_runner_classpath_mode%" == "argsfile" ]; then
294+
run_test_with_argument_file "$1"
295+
else
296+
create_and_run_classpath_jar "$2"
297+
fi
298+
}
299+
292300
function create_and_run_classpath_jar() {
293301
# Build class path as one single string separated by spaces
294302
MANIFEST_CLASSPATH=""
@@ -311,7 +319,7 @@ function create_and_run_classpath_jar() {
311319

312320
(
313321
echo "Manifest-Version: 1.0"
314-
322+
315323
CLASSPATH_LINE="Class-Path:$MANIFEST_CLASSPATH"
316324
# No line in the MANIFEST.MF file may be longer than 72 bytes.
317325
# A space prefix indicates the line is still the content of the last attribute.
@@ -338,6 +346,15 @@ function create_and_run_classpath_jar() {
338346
exit $exit_code
339347
}
340348

349+
function run_test_with_argument_file() {
350+
ARGS_FILE="$(mktemp -t XXXXXXXX.args)"
351+
echo "-cp "$CLASSPATH >> $ARGS_FILE
352+
$JAVABIN @$ARGS_FILE "${ARGS[@]}"
353+
exit_code=$?
354+
rm -f "$ARGS_FILE"
355+
exit $exit_code
356+
}
357+
341358
# If the user didn't specify a --classpath_limit, use the default value.
342359
if [ -z "$CLASSPATH_LIMIT" ]; then
343360
# Windows per-arg limit MAX_ARG_STRLEN == 8k
@@ -357,9 +374,9 @@ fi
357374
#Difference ends
358375

359376
if is_windows && (("${#CLASSPATH}" > ${CLASSPATH_LIMIT} )); then
360-
create_and_run_classpath_jar "${JARBIN}.exe"
377+
run_test_with_large_classpath "${JARBIN}.exe"
361378
elif (("${#CLASSPATH}" > ${CLASSPATH_LIMIT})); then
362-
create_and_run_classpath_jar "$JARBIN"
379+
run_test_with_large_classpath "$JARBIN"
363380
else
364381
exec $JAVABIN -classpath $CLASSPATH "${ARGS[@]}"
365382
fi

scala/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ toolchain_type(
1111
scala_toolchain(
1212
name = "default_toolchain_impl",
1313
scalacopts = [],
14+
use_argument_file_in_runner = True,
1415
visibility = ["//visibility:public"],
1516
)
1617

scala/private/phases/phase_write_executable.bzl

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,12 @@ def _write_executable_non_windows(ctx, executable, rjars, main_class, jvm_flags,
108108
wrapper.short_path,
109109
)
110110

111+
scala_toolchain = ctx.toolchains["//scala:toolchain_type"]
112+
113+
test_runner_classpath_mode = "argsfile" if scala_toolchain.use_argument_file_in_runner else "manifest"
114+
111115
if use_jacoco and ctx.configuration.coverage_enabled:
112-
jacocorunner = ctx.toolchains["@io_bazel_rules_scala//scala:toolchain_type"].jacocorunner
116+
jacocorunner = scala_toolchain.jacocorunner
113117
classpath = ctx.configuration.host_path_separator.join(
114118
["${RUNPATH}%s" % (j.short_path) for j in rjars.to_list() + jacocorunner.files.to_list()],
115119
)
@@ -137,6 +141,7 @@ def _write_executable_non_windows(ctx, executable, rjars, main_class, jvm_flags,
137141
"%set_jacoco_main_class%": """export JACOCO_MAIN_CLASS={}""".format(main_class),
138142
"%set_jacoco_java_runfiles_root%": """export JACOCO_JAVA_RUNFILES_ROOT=$JAVA_RUNFILES/{}/""".format(ctx.workspace_name),
139143
"%set_java_coverage_new_implementation%": """export JAVA_COVERAGE_NEW_IMPLEMENTATION=YES""",
144+
"%test_runner_classpath_mode%": test_runner_classpath_mode,
140145
},
141146
is_executable = True,
142147
)
@@ -163,6 +168,7 @@ def _write_executable_non_windows(ctx, executable, rjars, main_class, jvm_flags,
163168
"%set_jacoco_java_runfiles_root%": "",
164169
"%workspace_prefix%": ctx.workspace_name + "/",
165170
"%set_java_coverage_new_implementation%": """export JAVA_COVERAGE_NEW_IMPLEMENTATION=NO""",
171+
"%test_runner_classpath_mode%": test_runner_classpath_mode,
166172
},
167173
is_executable = True,
168174
)

scala/scala_toolchain.bzl

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ def _scala_toolchain_impl(ctx):
8484
enable_diagnostics_report = enable_diagnostics_report,
8585
jacocorunner = ctx.attr.jacocorunner,
8686
enable_stats_file = enable_stats_file,
87+
use_argument_file_in_runner = ctx.attr.use_argument_file_in_runner,
8788
)
8889
return [toolchain]
8990

@@ -137,6 +138,10 @@ scala_toolchain = rule(
137138
default = True,
138139
doc = "Enable writing of statsfile",
139140
),
141+
"use_argument_file_in_runner": attr.bool(
142+
default = False,
143+
doc = "Changes java binaries scripts (including tests) to use argument files and not classpath jars to improve performance, requires java > 8",
144+
),
140145
},
141146
fragments = ["java"],
142147
)

test/shell/test_scala_binary.sh

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,26 @@ test_scala_binary_expect_failure_on_missing_direct_deps_located_in_dependency_wh
1818
test_scala_library_expect_failure_on_missing_direct_deps ${dependency_target} ${test_target}
1919
}
2020

21+
test_scala_binary_allows_opt_in_to_use_of_argument_file_in_runner_for_improved_performance() {
22+
23+
bazel run --extra_toolchains="//test/toolchains:use_argument_file_in_runner" //test/src/main/scala/scalarules/test/large_classpath:largeClasspath
24+
grep "\"argsfile\" == \"argsfile\"" bazel-bin/test/src/main/scala/scalarules/test/large_classpath/largeClasspath
25+
RESPONSE_CODE=$?
26+
if [ $RESPONSE_CODE -ne 0 ]; then
27+
echo -e "${RED} Binary script does not use the argument file. $NC"
28+
exit -1
29+
fi
30+
31+
bazel run //test/src/main/scala/scalarules/test/large_classpath:largeClasspath
32+
grep "\"manifest\" == \"argsfile\"" bazel-bin/test/src/main/scala/scalarules/test/large_classpath/largeClasspath
33+
RESPONSE_CODE=$?
34+
if [ $RESPONSE_CODE -ne 0 ]; then
35+
echo -e "${RED} Binary script does not use the classpath jar. $NC"
36+
exit -1
37+
fi
38+
39+
}
40+
2141
$runner test_scala_binary_expect_failure_on_missing_direct_deps
2242
$runner test_scala_binary_expect_failure_on_missing_direct_deps_located_in_dependency_which_is_scala_binary
43+
$runner test_scala_binary_allows_opt_in_to_use_of_argument_file_in_runner_for_improved_performance

test/toolchains/BUILD.bazel

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,3 +117,16 @@ toolchain(
117117
toolchain_type = "@io_bazel_rules_scala//scala:toolchain_type",
118118
visibility = ["//visibility:public"],
119119
)
120+
121+
scala_toolchain(
122+
name = "use_argument_file_in_runner_impl",
123+
use_argument_file_in_runner = True,
124+
visibility = ["//visibility:public"],
125+
)
126+
127+
toolchain(
128+
name = "use_argument_file_in_runner",
129+
toolchain = "use_argument_file_in_runner_impl",
130+
toolchain_type = "@io_bazel_rules_scala//scala:toolchain_type",
131+
visibility = ["//visibility:public"],
132+
)

0 commit comments

Comments
 (0)