Skip to content

Commit

Permalink
Group synthetic classes with their context classes in DexFileSplitter…
Browse files Browse the repository at this point in the history
… 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
  • Loading branch information
ahumesky authored and copybara-github committed Jun 28, 2023
1 parent d00e34a commit 9092303
Show file tree
Hide file tree
Showing 6 changed files with 325 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,46 +42,61 @@ public void setUp() throws Exception {
public void testUnderLimit() {
DexLimitTracker tracker =
new DexLimitTracker(Math.max(dex.methodIds().size(), dex.fieldIds().size()));
assertThat(tracker.track(dex)).isFalse();
tracker.track(dex);
assertThat(tracker.outsideLimits()).isFalse();
}

@Test
public void testOverLimit() throws Exception {
DexLimitTracker tracker =
new DexLimitTracker(Math.max(dex.methodIds().size(), dex.fieldIds().size()) - 1);
assertThat(tracker.track(dex)).isTrue();
assertThat(tracker.track(dex)).isTrue();
assertThat(tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class)))).isTrue();
tracker.track(dex);
assertThat(tracker.outsideLimits()).isTrue();
tracker.track(dex);
assertThat(tracker.outsideLimits()).isTrue();
tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class)));
assertThat(tracker.outsideLimits()).isTrue();
}

@Test
public void testRepeatedReferencesDeduped() throws Exception {
DexLimitTracker tracker =
new DexLimitTracker(Math.max(dex.methodIds().size(), dex.fieldIds().size()));
assertThat(tracker.track(dex)).isFalse();
assertThat(tracker.track(dex)).isFalse();
assertThat(tracker.track(dex)).isFalse();
assertThat(tracker.track(dex)).isFalse();
assertThat(tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class)))).isTrue();
assertThat(tracker.track(dex)).isTrue();
tracker.track(dex);
assertThat(tracker.outsideLimits()).isFalse();
tracker.track(dex);
assertThat(tracker.outsideLimits()).isFalse();
tracker.track(dex);
assertThat(tracker.outsideLimits()).isFalse();
tracker.track(dex);
assertThat(tracker.outsideLimits()).isFalse();
tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class)));
assertThat(tracker.outsideLimits()).isTrue();
tracker.track(dex);
assertThat(tracker.outsideLimits()).isTrue();
}

@Test
public void testGoOverLimit() throws Exception {
DexLimitTracker tracker =
new DexLimitTracker(Math.max(dex.methodIds().size(), dex.fieldIds().size()));
assertThat(tracker.track(dex)).isFalse();
assertThat(tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class)))).isTrue();
tracker.track(dex);
assertThat(tracker.outsideLimits()).isFalse();
tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class)));
assertThat(tracker.outsideLimits()).isTrue();
}

@Test
public void testClear() throws Exception {
DexLimitTracker tracker =
new DexLimitTracker(Math.max(dex.methodIds().size(), dex.fieldIds().size()));
assertThat(tracker.track(dex)).isFalse();
assertThat(tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class)))).isTrue();
tracker.track(dex);
assertThat(tracker.outsideLimits()).isFalse();
tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class)));
assertThat(tracker.outsideLimits()).isTrue();
tracker.clear();
assertThat(tracker.track(dex)).isFalse();
tracker.track(dex);
assertThat(tracker.outsideLimits()).isFalse();
}

private static DexFile convertClass(Class<?> clazz) throws Exception {
Expand Down
14 changes: 14 additions & 0 deletions src/test/shell/bazel/android/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -184,3 +184,17 @@ android_sh_test(
"//src/test/shell/bazel:test-deps",
],
)

android_sh_test(
name = "DexFileSplitter_synthetic_classes_test",
size = "medium",
srcs = ["DexFileSplitter_synthetic_classes_test.sh"],
data = [
":android_helper",
"//external:android_sdk_for_testing",
"//src/test/shell/bazel:test-deps",
],
tags = [
"no_windows",
],
)
142 changes: 142 additions & 0 deletions src/test/shell/bazel/android/DexFileSplitter_synthetic_classes_test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
#!/bin/bash
#
# 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.

# For these tests to run do the following:
#
# 1. Install an Android SDK from https://developer.android.com
# 2. Set the $ANDROID_HOME environment variable
# 3. Uncomment the line in WORKSPACE containing android_sdk_repository
#
# Note that if the environment is not set up as above android_integration_test
# will silently be ignored and will be shown as passing.

# Load the test setup defined in the parent directory
CURRENT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"

source "${CURRENT_DIR}/android_helper.sh" \
|| { echo "android_helper.sh not found!" >&2; exit 1; }
fail_if_no_android_sdk

source "${CURRENT_DIR}/../../integration_test_setup.sh" \
|| { echo "integration_test_setup.sh not found!" >&2; exit 1; }

resolve_android_toolchains "$1"

function test_DexFileSplitter_synthetic_classes_crossing_dexfiles() {
create_new_workspace
setup_android_sdk_support

mkdir -p java/com/testapp

cat > java/com/testapp/AndroidManifest.xml <<EOF
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
package="com.testapp"
android:versionCode="1"
android:versionName="1.0" >
<uses-sdk
android:minSdkVersion="30"
android:targetSdkVersion="30" />
<application android:label="Test App" >
<activity
android:name="com.testapp.MainActivity"
android:label="App"
android:exported="true">
<intent-filter>
<action android:name="android.intent.action.MAIN" />
<category android:name="android.intent.category.LAUNCHER" />
</intent-filter>
</activity>
</application>
</manifest>
EOF

cat > java/com/testapp/MainActivity.java <<EOF
package com.testapp;
import android.app.Activity;
import android.os.Bundle;
import android.util.Log;
public class MainActivity extends Activity {
@Override
public void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
Log.i("tag", "info");
}
}
EOF

generate_java_file_with_many_synthetic_classes > java/com/testapp/BigLib.java

cat > java/com/testapp/BUILD <<EOF
android_binary(
name = "testapp",
srcs = [
"MainActivity.java",
":BigLib.java",
],
manifest = "AndroidManifest.xml",
)
EOF

bazel build java/com/testapp || fail "Test app should have built succesfully"

dex_file_count=$(unzip -l bazel-bin/java/com/testapp/testapp.apk | grep "classes[0-9]*.dex" | wc -l)
if [[ ! "$dex_file_count" -ge "2" ]]; then
echo "Expected at least 2 dexes in app, found: $dex_file_count"
exit 1
fi
}


function generate_java_file_with_many_synthetic_classes() {
echo "package com.testapp;"
echo "public class BigLib {"

# First generate enough inner classes to fill up most of the dex
for (( i = 0; i < 21400; i++ )) do

echo " public static class Bar$i {"
echo " public int bar() {"
echo " return $i;"
echo " }"
echo " }"

done

# Then create enough synethetic classes via lambdas to fill up the rest of the
# dex and into another dex file.
echo " public interface IntSupplier {"
echo " int supply();"
echo " }"

echo " public static class Foo {"
echo " public IntSupplier[] foo() {"
echo " return new IntSupplier[] {"

for ((i = 0; i < 6000; i++ )) do
echo " () -> $i,"
done

echo " };"
echo " }"
echo " }"
echo "}"
}

run_suite "Tests for DexFileSplitter with synthetic classes crossing dexfiles"
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ public DexFileAggregator add(Dex dexFile) {
if (multidex.isMultidexAllowed()) {
// To determine whether currentShard is "full" we track unique field and method signatures,
// which predicts precisely the number of field and method indices.
if (tracker.track(dexFile) && !currentShard.isEmpty()) {
tracker.track(dexFile);
if (tracker.outsideLimits() && !currentShard.isEmpty()) {
// For simplicity just start a new shard to fit the given file.
// Don't bother with waiting for a later file that might fit the old shard as in the extreme
// we'd have to wait until the end to write all shards.
Expand Down
Loading

0 comments on commit 9092303

Please sign in to comment.