Skip to content

Commit 9092303

Browse files
ahumeskycopybara-github
authored andcommitted
Group synthetic classes with their context classes in DexFileSplitter so that synthetic classes don't end up without their context classes in dex shards, which will causing merging to fail in DexFileMerger. Fixes #16368.
RELNOTES: None PiperOrigin-RevId: 544134712 Change-Id: Ib29f6659f18dd71be96a7985bc25cfb44e719ae5
1 parent d00e34a commit 9092303

File tree

6 files changed

+325
-54
lines changed

6 files changed

+325
-54
lines changed

src/test/java/com/google/devtools/build/android/dexer/DexLimitTrackerTest.java

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -42,46 +42,61 @@ public void setUp() throws Exception {
4242
public void testUnderLimit() {
4343
DexLimitTracker tracker =
4444
new DexLimitTracker(Math.max(dex.methodIds().size(), dex.fieldIds().size()));
45-
assertThat(tracker.track(dex)).isFalse();
45+
tracker.track(dex);
46+
assertThat(tracker.outsideLimits()).isFalse();
4647
}
4748

4849
@Test
4950
public void testOverLimit() throws Exception {
5051
DexLimitTracker tracker =
5152
new DexLimitTracker(Math.max(dex.methodIds().size(), dex.fieldIds().size()) - 1);
52-
assertThat(tracker.track(dex)).isTrue();
53-
assertThat(tracker.track(dex)).isTrue();
54-
assertThat(tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class)))).isTrue();
53+
tracker.track(dex);
54+
assertThat(tracker.outsideLimits()).isTrue();
55+
tracker.track(dex);
56+
assertThat(tracker.outsideLimits()).isTrue();
57+
tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class)));
58+
assertThat(tracker.outsideLimits()).isTrue();
5559
}
5660

5761
@Test
5862
public void testRepeatedReferencesDeduped() throws Exception {
5963
DexLimitTracker tracker =
6064
new DexLimitTracker(Math.max(dex.methodIds().size(), dex.fieldIds().size()));
61-
assertThat(tracker.track(dex)).isFalse();
62-
assertThat(tracker.track(dex)).isFalse();
63-
assertThat(tracker.track(dex)).isFalse();
64-
assertThat(tracker.track(dex)).isFalse();
65-
assertThat(tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class)))).isTrue();
66-
assertThat(tracker.track(dex)).isTrue();
65+
tracker.track(dex);
66+
assertThat(tracker.outsideLimits()).isFalse();
67+
tracker.track(dex);
68+
assertThat(tracker.outsideLimits()).isFalse();
69+
tracker.track(dex);
70+
assertThat(tracker.outsideLimits()).isFalse();
71+
tracker.track(dex);
72+
assertThat(tracker.outsideLimits()).isFalse();
73+
tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class)));
74+
assertThat(tracker.outsideLimits()).isTrue();
75+
tracker.track(dex);
76+
assertThat(tracker.outsideLimits()).isTrue();
6777
}
6878

6979
@Test
7080
public void testGoOverLimit() throws Exception {
7181
DexLimitTracker tracker =
7282
new DexLimitTracker(Math.max(dex.methodIds().size(), dex.fieldIds().size()));
73-
assertThat(tracker.track(dex)).isFalse();
74-
assertThat(tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class)))).isTrue();
83+
tracker.track(dex);
84+
assertThat(tracker.outsideLimits()).isFalse();
85+
tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class)));
86+
assertThat(tracker.outsideLimits()).isTrue();
7587
}
7688

7789
@Test
7890
public void testClear() throws Exception {
7991
DexLimitTracker tracker =
8092
new DexLimitTracker(Math.max(dex.methodIds().size(), dex.fieldIds().size()));
81-
assertThat(tracker.track(dex)).isFalse();
82-
assertThat(tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class)))).isTrue();
93+
tracker.track(dex);
94+
assertThat(tracker.outsideLimits()).isFalse();
95+
tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class)));
96+
assertThat(tracker.outsideLimits()).isTrue();
8397
tracker.clear();
84-
assertThat(tracker.track(dex)).isFalse();
98+
tracker.track(dex);
99+
assertThat(tracker.outsideLimits()).isFalse();
85100
}
86101

87102
private static DexFile convertClass(Class<?> clazz) throws Exception {

src/test/shell/bazel/android/BUILD

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,3 +184,17 @@ android_sh_test(
184184
"//src/test/shell/bazel:test-deps",
185185
],
186186
)
187+
188+
android_sh_test(
189+
name = "DexFileSplitter_synthetic_classes_test",
190+
size = "medium",
191+
srcs = ["DexFileSplitter_synthetic_classes_test.sh"],
192+
data = [
193+
":android_helper",
194+
"//external:android_sdk_for_testing",
195+
"//src/test/shell/bazel:test-deps",
196+
],
197+
tags = [
198+
"no_windows",
199+
],
200+
)
Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
#!/bin/bash
2+
#
3+
# Copyright 2023 The Bazel Authors. All rights reserved.
4+
#
5+
# Licensed under the Apache License, Version 2.0 (the "License");
6+
# you may not use this file except in compliance with the License.
7+
# You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing, software
12+
# distributed under the License is distributed on an "AS IS" BASIS,
13+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
# See the License for the specific language governing permissions and
15+
# limitations under the License.
16+
17+
# For these tests to run do the following:
18+
#
19+
# 1. Install an Android SDK from https://developer.android.com
20+
# 2. Set the $ANDROID_HOME environment variable
21+
# 3. Uncomment the line in WORKSPACE containing android_sdk_repository
22+
#
23+
# Note that if the environment is not set up as above android_integration_test
24+
# will silently be ignored and will be shown as passing.
25+
26+
# Load the test setup defined in the parent directory
27+
CURRENT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
28+
29+
source "${CURRENT_DIR}/android_helper.sh" \
30+
|| { echo "android_helper.sh not found!" >&2; exit 1; }
31+
fail_if_no_android_sdk
32+
33+
source "${CURRENT_DIR}/../../integration_test_setup.sh" \
34+
|| { echo "integration_test_setup.sh not found!" >&2; exit 1; }
35+
36+
resolve_android_toolchains "$1"
37+
38+
function test_DexFileSplitter_synthetic_classes_crossing_dexfiles() {
39+
create_new_workspace
40+
setup_android_sdk_support
41+
42+
mkdir -p java/com/testapp
43+
44+
cat > java/com/testapp/AndroidManifest.xml <<EOF
45+
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
46+
package="com.testapp"
47+
android:versionCode="1"
48+
android:versionName="1.0" >
49+
50+
<uses-sdk
51+
android:minSdkVersion="30"
52+
android:targetSdkVersion="30" />
53+
54+
<application android:label="Test App" >
55+
<activity
56+
android:name="com.testapp.MainActivity"
57+
android:label="App"
58+
android:exported="true">
59+
<intent-filter>
60+
<action android:name="android.intent.action.MAIN" />
61+
<category android:name="android.intent.category.LAUNCHER" />
62+
</intent-filter>
63+
</activity>
64+
</application>
65+
</manifest>
66+
EOF
67+
68+
cat > java/com/testapp/MainActivity.java <<EOF
69+
package com.testapp;
70+
71+
import android.app.Activity;
72+
import android.os.Bundle;
73+
import android.util.Log;
74+
75+
public class MainActivity extends Activity {
76+
@Override
77+
public void onCreate(Bundle savedInstanceState) {
78+
super.onCreate(savedInstanceState);
79+
Log.i("tag", "info");
80+
}
81+
}
82+
EOF
83+
84+
generate_java_file_with_many_synthetic_classes > java/com/testapp/BigLib.java
85+
86+
cat > java/com/testapp/BUILD <<EOF
87+
android_binary(
88+
name = "testapp",
89+
srcs = [
90+
"MainActivity.java",
91+
":BigLib.java",
92+
],
93+
manifest = "AndroidManifest.xml",
94+
)
95+
EOF
96+
97+
bazel build java/com/testapp || fail "Test app should have built succesfully"
98+
99+
dex_file_count=$(unzip -l bazel-bin/java/com/testapp/testapp.apk | grep "classes[0-9]*.dex" | wc -l)
100+
if [[ ! "$dex_file_count" -ge "2" ]]; then
101+
echo "Expected at least 2 dexes in app, found: $dex_file_count"
102+
exit 1
103+
fi
104+
}
105+
106+
107+
function generate_java_file_with_many_synthetic_classes() {
108+
echo "package com.testapp;"
109+
echo "public class BigLib {"
110+
111+
# First generate enough inner classes to fill up most of the dex
112+
for (( i = 0; i < 21400; i++ )) do
113+
114+
echo " public static class Bar$i {"
115+
echo " public int bar() {"
116+
echo " return $i;"
117+
echo " }"
118+
echo " }"
119+
120+
done
121+
122+
# Then create enough synethetic classes via lambdas to fill up the rest of the
123+
# dex and into another dex file.
124+
echo " public interface IntSupplier {"
125+
echo " int supply();"
126+
echo " }"
127+
128+
echo " public static class Foo {"
129+
echo " public IntSupplier[] foo() {"
130+
echo " return new IntSupplier[] {"
131+
132+
for ((i = 0; i < 6000; i++ )) do
133+
echo " () -> $i,"
134+
done
135+
136+
echo " };"
137+
echo " }"
138+
echo " }"
139+
echo "}"
140+
}
141+
142+
run_suite "Tests for DexFileSplitter with synthetic classes crossing dexfiles"

src/tools/android/java/com/google/devtools/build/android/dexer/DexFileAggregator.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,8 @@ public DexFileAggregator add(Dex dexFile) {
8282
if (multidex.isMultidexAllowed()) {
8383
// To determine whether currentShard is "full" we track unique field and method signatures,
8484
// which predicts precisely the number of field and method indices.
85-
if (tracker.track(dexFile) && !currentShard.isEmpty()) {
85+
tracker.track(dexFile);
86+
if (tracker.outsideLimits() && !currentShard.isEmpty()) {
8687
// For simplicity just start a new shard to fit the given file.
8788
// Don't bother with waiting for a later file that might fit the old shard as in the extreme
8889
// we'd have to wait until the end to write all shards.

0 commit comments

Comments
 (0)