Skip to content

Commit 244ec80

Browse files
ted-xiecopybara-github
authored andcommitted
Use the C++ singlejar in ZipFilterAction instead of legacy Java impl
ZipFilterAction currently depends on ZipCombiner and several other utility classes from the now-deleted Bazel Java singlejar tool (it was rewritten in C++ a while ago). ZipFilterAction doesn't actually need the advanced filtering and callback capabilities of ZipCombiner, since it only cares about a single zip archive at a time. This change refactors ZipFilterAction to use the C++ singlejar under the hood. Internal testing suggests a 25% wall time reduction in zip filtering is achievable. PiperOrigin-RevId: 775670271 Change-Id: I5534b5b1c98f06ad3905a06d86ca339286c3a7c2
1 parent 0fecacd commit 244ec80

10 files changed

Lines changed: 162 additions & 179 deletions

File tree

.bazelrc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
common --java_runtime_version=17
2+
common --tool_java_language_version=17
23

34
# Workaround for a rules_java + bazel < 8.3.0 issue. It should only be relevant
45
# for bazel@HEAD and rolling releases.

MODULE.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ module(
66

77
bazel_dep(name = "platforms", version = "0.0.5")
88
bazel_dep(name = "rules_license", version = "1.0.0")
9-
bazel_dep(name = "rules_java", version = "8.12.0")
9+
bazel_dep(name = "rules_java", version = "8.13.0")
1010
bazel_dep(name = "rules_cc", version = "0.1.1")
1111
bazel_dep(name = "rules_shell", version = "0.1.2")
1212

examples/basicapp/.bazelrc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ common:core_library_desugaring --desugar_java8_libs
33

44
# Flags to enable mobile-install v3
55
mobile-install --mode=skylark --mobile_install_aspect=@rules_android//mobile_install:mi.bzl --mobile_install_supported_rules=android_binary
6+
# Required to build Android builder tools
7+
common --tool_java_language_version=17
68
# Required to invoke the Studio deployer jar
79
common --tool_java_runtime_version=17
810

examples/basicapp/java/com/basicapp/BUILD

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ android_binary(
99
android_library(
1010
name = "basic_lib",
1111
srcs = ["BasicActivity.java"],
12-
javacopts = ["-source 11 -target 11"],
1312
manifest = "AndroidManifest.xml",
1413
resource_files = glob(["res/**"]),
1514
)

prereqs.bzl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@ def rules_android_prereqs(dev_mode = False):
2525
http_archive,
2626
name = "rules_java",
2727
urls = [
28-
"https://github.com/bazelbuild/rules_java/releases/download/8.12.0/rules_java-8.12.0.tar.gz",
28+
"https://github.com/bazelbuild/rules_java/releases/download/8.13.0/rules_java-8.13.0.tar.gz",
2929
],
30-
sha256 = "1558508fc6c348d7f99477bd21681e5746936f15f0436b5f4233e30832a590f9",
30+
sha256 = "b6c6d92ca9dbb77de31fb6c6a794d20427072663ce41c2b047902ffcc123e3ef",
3131
)
3232

3333
maybe(

src/tools/java/com/google/devtools/build/android/BUILD

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,30 +19,22 @@ java_binary(
1919

2020
java_binary(
2121
name = "ZipFilterAction",
22+
data = [
23+
"//tools/jdk:singlejar",
24+
],
2225
# Memory consumption of SingleJar is about 250 bytes per entry in the output file. Unfortunately,
2326
# the JVM tends to kill the process with an OOM long before we're at the limit. In the most
2427
# recent example, 400 MB of memory was enough for about 500,000 entries.
25-
jvm_flags = ["-Xmx1600m"],
28+
jvm_flags = [
29+
"-Xmx1600m",
30+
"-Drunfiles.path=$$0.runfiles",
31+
"-Dsinglejar.path=$(rlocationpath //tools/jdk:singlejar)",
32+
],
2633
main_class = "com.google.devtools.build.android.ZipFilterAction",
2734
visibility = ["//tools/android:__pkg__"],
2835
runtime_deps = [":android_builder_lib"],
2936
)
3037

31-
run_singlejar(
32-
name = "singlejar_jar_gen",
33-
srcs = ["@android_tools//:all_android_tools_deploy.jar"],
34-
out = "singlejar.jar",
35-
include_prefixes = [
36-
"com/google/devtools/build/singlejar/",
37-
"com/google/devtools/build/zip/",
38-
],
39-
)
40-
41-
java_import(
42-
name = "singlejar_jar",
43-
jars = [":singlejar_jar_gen"],
44-
)
45-
4638
run_singlejar(
4739
name = "databinding_exec_jar_gen",
4840
srcs = ["@android_tools//:all_android_tools_deploy.jar"],
@@ -108,7 +100,6 @@ java_library(
108100
":android_databinding_wrapper_lib",
109101
":android_options_utils",
110102
":dependency_info",
111-
":singlejar_jar",
112103
"//src/tools/java/com/google/devtools/build/android/junctions",
113104
"//src/tools/java/com/google/devtools/build/android/proto:resources_java_proto",
114105
"//src/tools/java/com/google/devtools/build/android/proto:serialize_format_java_pb",
@@ -154,8 +145,6 @@ java_library(
154145
],
155146
)
156147

157-
158-
159148
java_library(
160149
name = "android_common_30_1_3",
161150
# Do not sort: Deps order matters until android_tools.jar deps are deleted (b/393172052).

src/tools/java/com/google/devtools/build/android/ZipFilterAction.java

Lines changed: 136 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
// limitations under the License.
1414
package com.google.devtools.build.android;
1515

16+
import static java.nio.charset.StandardCharsets.UTF_8;
17+
1618
import com.beust.jcommander.IStringConverter;
1719
import com.beust.jcommander.IValueValidator;
1820
import com.beust.jcommander.JCommander;
@@ -23,16 +25,18 @@
2325
import com.google.common.base.Joiner;
2426
import com.google.common.base.Stopwatch;
2527
import com.google.common.collect.ImmutableList;
26-
import com.google.common.collect.ImmutableMap;
2728
import com.google.common.collect.ImmutableSetMultimap;
2829
import com.google.common.collect.Multimap;
29-
import com.google.devtools.build.singlejar.ZipCombiner;
30-
import com.google.devtools.build.singlejar.ZipCombiner.OutputMode;
30+
import java.io.BufferedReader;
31+
import java.io.BufferedWriter;
32+
import java.io.File;
33+
import java.io.FileOutputStream;
3134
import java.io.IOException;
32-
import java.io.OutputStream;
35+
import java.io.InputStreamReader;
3336
import java.nio.file.FileSystems;
3437
import java.nio.file.Files;
3538
import java.nio.file.Path;
39+
import java.util.ArrayList;
3640
import java.util.Collection;
3741
import java.util.Enumeration;
3842
import java.util.LinkedHashSet;
@@ -43,6 +47,7 @@
4347
import java.util.regex.Pattern;
4448
import java.util.zip.ZipEntry;
4549
import java.util.zip.ZipFile;
50+
import java.util.zip.ZipOutputStream;
4651

4752
/**
4853
* Action to filter entries out of a Zip file.
@@ -71,6 +76,23 @@
7176
* </pre>
7277
*/
7378
public class ZipFilterAction {
79+
/** A copy of Bazel's singlejar.ZipCombiner#OutputMode enum. */
80+
enum OutputMode {
81+
82+
/** Output entries using any method. */
83+
DONT_CARE,
84+
85+
/**
86+
* Output all entries using DEFLATE method, except directory entries. It is always more
87+
* efficient to store directory entries uncompressed.
88+
*/
89+
FORCE_DEFLATE,
90+
91+
/** Output all entries using STORED method. */
92+
FORCE_STORED,
93+
}
94+
95+
record GenerateExcludeListResult(int sawErrors, ArrayList<String> excludeList) {}
7496

7597
private static final Logger logger = Logger.getLogger(ZipFilterAction.class.getName());
7698

@@ -200,45 +222,133 @@ public static void main(String[] args) throws IOException {
200222
System.exit(run(args));
201223
}
202224

203-
static int run(String[] args) throws IOException {
204-
Options options = new Options();
205-
new JCommander(options).parse(args);
206-
logger.fine(
207-
String.format(
208-
"Creating filter from entries of type %s, in zip files %s.",
209-
options.filterTypes, options.filterZips));
210-
211-
final Stopwatch timer = Stopwatch.createStarted();
225+
static GenerateExcludeListResult generateExcludeList(Options options, Stopwatch timer)
226+
throws IOException {
212227
Multimap<String, Long> entriesToOmit =
213228
getEntriesToOmit(options.filterZips, options.filterTypes);
214229
final String explicitFilter =
215230
options.explicitFilters.isEmpty()
216231
? ""
217232
: String.format(".*(%s).*", Joiner.on("|").join(options.explicitFilters));
218233
logger.fine(String.format("Filter created in %dms", timer.elapsed(TimeUnit.MILLISECONDS)));
234+
ArrayList<String> excludeList = new ArrayList<>();
219235

220-
ImmutableMap.Builder<String, Long> inputEntries = ImmutableMap.builder();
221-
try (ZipFile zf = new ZipFile(options.inputZip.toFile())) {
236+
int sawErrors = 0;
237+
try (ZipFile zf = new ZipFile(options.inputZip.toFile());
238+
ZipOutputStream zos =
239+
new ZipOutputStream(new FileOutputStream(options.outputZip.toString()))) {
222240
Enumeration<? extends ZipEntry> entries = zf.entries();
223241

224242
while (entries.hasMoreElements()) {
225243
ZipEntry entry = entries.nextElement();
226-
inputEntries.put(entry.getName(), entry.getCrc());
244+
if (entry.getName().matches(explicitFilter)) {
245+
excludeList.add(entry.getName());
246+
} else if (entriesToOmit.containsEntry(entry.getName(), entry.getCrc())) {
247+
excludeList.add(entry.getName());
248+
} else if (entriesToOmit.containsKey(entry.getName())) {
249+
// entriesToOmit contains the filename, but a different CRC was observed.
250+
if (options.hashMismatchCheckMode == HashMismatchCheckMode.IGNORE) {
251+
// Just add it to the excluded entry list.
252+
excludeList.add(entry.getName());
253+
} else {
254+
if (options.hashMismatchCheckMode == HashMismatchCheckMode.ERROR) {
255+
logger.severe(
256+
String.format(
257+
"ERROR: Requested to filter entries of name "
258+
+ "'%s'; name matches but the hash does not.\n",
259+
entry.getName()));
260+
sawErrors = 1;
261+
excludeList.add(entry.getName());
262+
} else {
263+
logger.severe(
264+
String.format(
265+
"WARNING: Requested to filter entries of name "
266+
+ "'%s'; name matches but the hash does not. Copying anyway.\n",
267+
entry.getName()));
268+
}
269+
}
270+
}
227271
}
228272
}
273+
return new GenerateExcludeListResult(sawErrors, excludeList);
274+
}
229275

230-
ZipFilterEntryFilter entryFilter =
231-
new ZipFilterEntryFilter(
232-
explicitFilter,
233-
entriesToOmit,
234-
inputEntries.buildOrThrow(),
235-
options.hashMismatchCheckMode);
276+
@SuppressWarnings("RuntimeExec")
277+
static int run(String[] args) throws IOException {
278+
Options options = new Options();
279+
new JCommander(options).parse(args);
280+
logger.fine(
281+
String.format(
282+
"Creating filter from entries of type %s, in zip files %s.",
283+
options.filterTypes, options.filterZips));
284+
285+
String compressionStrategy = "--dont_change_compression";
286+
if (options.outputMode == OutputMode.FORCE_STORED) {
287+
compressionStrategy = "--compression";
288+
} else if (options.outputMode == OutputMode.FORCE_DEFLATE) {
289+
throw new IllegalArgumentException("FORCE_DEFLATE is not supported.");
290+
}
291+
292+
String singleJarPath =
293+
Path.of(System.getProperty("runfiles.path"), System.getProperty("singlejar.path"))
294+
.toString();
295+
ImmutableList.Builder<String> singleJarArgsBuilder = ImmutableList.builder();
296+
singleJarArgsBuilder
297+
.add("--sources")
298+
.add(options.inputZip.toString())
299+
.add("--output")
300+
.add(options.outputZip.toString())
301+
.add(compressionStrategy)
302+
.add("--exclude_build_data")
303+
.add("--normalize");
304+
305+
final Stopwatch timer = Stopwatch.createStarted();
306+
GenerateExcludeListResult excludeListResult = generateExcludeList(options, timer);
307+
int sawErrors = excludeListResult.sawErrors();
308+
ArrayList<String> excludeList = excludeListResult.excludeList();
309+
310+
if (!excludeList.isEmpty()) {
311+
singleJarArgsBuilder.add("--exclude_zip_entries").addAll(excludeList);
312+
}
313+
314+
ImmutableList<String> singleJarArgs = singleJarArgsBuilder.build();
315+
// Dump the singlejar args into a params file
316+
File paramsFile = File.createTempFile("singlejar_params", ".txt");
317+
try (BufferedWriter writer = Files.newBufferedWriter(paramsFile.toPath(), UTF_8)) {
318+
for (String arg : singleJarArgs) {
319+
writer.write(arg + "\n");
320+
}
321+
}
236322

237-
try (OutputStream out = Files.newOutputStream(options.outputZip);
238-
ZipCombiner combiner = new ZipCombiner(options.outputMode, entryFilter, out)) {
239-
combiner.addZip(options.inputZip.toFile());
323+
boolean singleJarError = false;
324+
Process singleJarProcess =
325+
Runtime.getRuntime().exec(new String[] {singleJarPath, "@" + paramsFile.getAbsolutePath()});
326+
try {
327+
int singlejarExitCode = singleJarProcess.waitFor();
328+
if (singlejarExitCode != 0) {
329+
sawErrors = 1;
330+
singleJarError = true;
331+
logger.severe(
332+
String.format(
333+
"ERROR: singlejar failed with exit code %d. See logs for details.",
334+
singlejarExitCode));
335+
}
336+
} catch (InterruptedException e) {
337+
singleJarError = true;
338+
logger.severe(String.format("ERROR: singlejar was interrupted: %s", e.getMessage()));
339+
sawErrors = 1;
340+
}
341+
// Dump out the singlejar stderr if there was an issue.
342+
if (singleJarError) {
343+
InputStreamReader isr = new InputStreamReader(singleJarProcess.getErrorStream(), UTF_8);
344+
BufferedReader br = new BufferedReader(isr);
345+
String line;
346+
while ((line = br.readLine()) != null) {
347+
System.out.println(" [singlejar stderr] " + line);
348+
}
240349
}
241350
logger.fine(String.format("Filtering completed in %dms", timer.elapsed(TimeUnit.MILLISECONDS)));
242-
return entryFilter.sawErrors() ? 1 : 0;
351+
352+
return sawErrors;
243353
}
244354
}

0 commit comments

Comments
 (0)