diff options
41 files changed, 701 insertions, 291 deletions
diff --git a/artd/artd.cc b/artd/artd.cc index b4cbbcce8d..b9636a9e54 100644 --- a/artd/artd.cc +++ b/artd/artd.cc @@ -775,6 +775,12 @@ ndk::ScopedAStatus Artd::getDexoptNeeded(const std::string& in_dexFile, _aidl_return->isVdexUsable = status.IsVdexUsable(); _aidl_return->artifactsLocation = ArtifactsLocationToAidl(status.GetLocation()); + std::optional<bool> has_dex_files = oat_file_assistant->HasDexFiles(&error_msg); + if (!has_dex_files.has_value()) { + return NonFatal("Failed to open dex file: " + error_msg); + } + _aidl_return->hasDexCode = *has_dex_files; + return ScopedAStatus::ok(); } diff --git a/artd/binder/com/android/server/art/GetDexoptNeededResult.aidl b/artd/binder/com/android/server/art/GetDexoptNeededResult.aidl index 99c4951d10..8e361b48f1 100644 --- a/artd/binder/com/android/server/art/GetDexoptNeededResult.aidl +++ b/artd/binder/com/android/server/art/GetDexoptNeededResult.aidl @@ -28,7 +28,14 @@ parcelable GetDexoptNeededResult { boolean isVdexUsable; /** The location of the best usable artifacts. */ ArtifactsLocation artifactsLocation = ArtifactsLocation.NONE_OR_ERROR; + /** + * True if the dex file has dex code. (The dex file is a .jar/.apk file that has .dex entries, + * or is a .dex file.) False otherwise. (The dex file is a .jar/.apk file that has no .dex + * entries.) + */ + boolean hasDexCode; + @Backing(type="int") enum ArtifactsLocation { /** No usable artifacts. */ NONE_OR_ERROR = 0, diff --git a/build/apex/art_apex_test.py b/build/apex/art_apex_test.py index c2549cc93e..e10d979837 100755 --- a/build/apex/art_apex_test.py +++ b/build/apex/art_apex_test.py @@ -563,6 +563,7 @@ class ReleaseTargetChecker: # Check internal Java libraries self._checker.check_java_library("service-art") + self._checker.check_file('javalib/service-art.jar.prof') # Check exported native libraries for Managed Core Library. self._checker.check_native_library('libandroidio') diff --git a/libartpalette/Android.bp b/libartpalette/Android.bp index e6aae64b64..6e7415e7b1 100644 --- a/libartpalette/Android.bp +++ b/libartpalette/Android.bp @@ -52,7 +52,7 @@ art_cc_library { // libdexfile hence depends on the source instead. // TODO(b/172480617): Alternatively, clean up when we no longer need to // support both prebuilts and sources present simultaneously. - "//prebuilts/module_sdk/art/current/host-exports", + "//prebuilts/module_sdk/art:__subpackages__", ], header_libs: [ "libbase_headers", diff --git a/libartservice/service/Android.bp b/libartservice/service/Android.bp index ad2033ab35..7faa20d2fa 100644 --- a/libartservice/service/Android.bp +++ b/libartservice/service/Android.bp @@ -116,6 +116,9 @@ java_sdk_library { "framework-system-server-module-optimize-defaults", ], permitted_packages: ["com.android.server.art"], + dex_preopt: { + profile: "art-profile", + }, visibility: [ "//art:__subpackages__", "//frameworks/base/services/core", diff --git a/libartservice/service/art-profile b/libartservice/service/art-profile new file mode 100644 index 0000000000..4c6562fe0f --- /dev/null +++ b/libartservice/service/art-profile @@ -0,0 +1,16 @@ +# +# Copyright (C) 2017 The Android Open Source Project +# +# 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. +# + diff --git a/libartservice/service/java/com/android/server/art/BackgroundDexoptJob.java b/libartservice/service/java/com/android/server/art/BackgroundDexoptJob.java index b2973260c4..9944e3fb1f 100644 --- a/libartservice/service/java/com/android/server/art/BackgroundDexoptJob.java +++ b/libartservice/service/java/com/android/server/art/BackgroundDexoptJob.java @@ -228,43 +228,13 @@ public class BackgroundDexoptJob { synchronized (this) { stopReason = mLastStopReason; } - if (result instanceof CompletedResult) { - var completedResult = (CompletedResult) result; - ArtStatsLog.write(ArtStatsLog.BACKGROUND_DEXOPT_JOB_ENDED, - getStatusForStats(completedResult, stopReason), - stopReason.orElse(JobParameters.STOP_REASON_UNDEFINED), - completedResult.durationMs(), 0 /* deprecated */); + if (result instanceof CompletedResult completedResult) { + BackgroundDexoptJobStatsReporter.reportSuccess(completedResult, stopReason); } else if (result instanceof FatalErrorResult) { - ArtStatsLog.write(ArtStatsLog.BACKGROUND_DEXOPT_JOB_ENDED, - ArtStatsLog.BACKGROUND_DEXOPT_JOB_ENDED__STATUS__STATUS_FATAL_ERROR, - JobParameters.STOP_REASON_UNDEFINED, 0 /* durationMs */, 0 /* deprecated */); + BackgroundDexoptJobStatsReporter.reportFailure(); } } - private int getStatusForStats(@NonNull CompletedResult result, Optional<Integer> stopReason) { - if (result.dexoptResult().getFinalStatus() == DexoptResult.DEXOPT_CANCELLED) { - if (stopReason.isPresent()) { - return ArtStatsLog - .BACKGROUND_DEXOPT_JOB_ENDED__STATUS__STATUS_ABORT_BY_CANCELLATION; - } else { - return ArtStatsLog.BACKGROUND_DEXOPT_JOB_ENDED__STATUS__STATUS_ABORT_BY_API; - } - } - - boolean isSkippedDueToStorageLow = - result.dexoptResult() - .getPackageDexoptResults() - .stream() - .flatMap(packageResult - -> packageResult.getDexContainerFileDexoptResults().stream()) - .anyMatch(fileResult -> fileResult.isSkippedDueToStorageLow()); - if (isSkippedDueToStorageLow) { - return ArtStatsLog.BACKGROUND_DEXOPT_JOB_ENDED__STATUS__STATUS_ABORT_NO_SPACE_LEFT; - } - - return ArtStatsLog.BACKGROUND_DEXOPT_JOB_ENDED__STATUS__STATUS_JOB_FINISHED; - } - static abstract class Result {} static class FatalErrorResult extends Result {} diff --git a/libartservice/service/java/com/android/server/art/BackgroundDexoptJobStatsReporter.java b/libartservice/service/java/com/android/server/art/BackgroundDexoptJobStatsReporter.java new file mode 100644 index 0000000000..533e3408e3 --- /dev/null +++ b/libartservice/service/java/com/android/server/art/BackgroundDexoptJobStatsReporter.java @@ -0,0 +1,109 @@ +package com.android.server.art; + +import android.annotation.NonNull; +import android.app.job.JobParameters; +import android.os.Build; + +import androidx.annotation.RequiresApi; + +import com.android.server.art.model.DexoptResult; + +import dalvik.system.DexFile; + +import java.util.List; +import java.util.Optional; +import java.util.stream.Collectors; + +/** + * This is an helper class to report the background DexOpt job metrics to StatsD. + * + * @hide + */ +@RequiresApi(Build.VERSION_CODES.UPSIDE_DOWN_CAKE) +public class BackgroundDexoptJobStatsReporter { + public static void reportFailure() { + ArtStatsLog.write(ArtStatsLog.BACKGROUND_DEXOPT_JOB_ENDED, + ArtStatsLog.BACKGROUND_DEXOPT_JOB_ENDED__STATUS__STATUS_FATAL_ERROR, + JobParameters.STOP_REASON_UNDEFINED, 0 /* durationMs */, 0 /* deprecated */, + 0 /* optimizedPackagesCount */, 0 /* packagesDependingOnBootClasspathCount */, + 0 /* totalPackagesCount */); + } + + public static void reportSuccess(@NonNull BackgroundDexoptJob.CompletedResult completedResult, + Optional<Integer> stopReason) { + List<DexoptResult.PackageDexoptResult> packageDexoptResults = + getFilteredPackageResults(completedResult); + ArtStatsLog.write(ArtStatsLog.BACKGROUND_DEXOPT_JOB_ENDED, + getStatusForStats(completedResult, stopReason), + stopReason.orElse(JobParameters.STOP_REASON_UNDEFINED), + completedResult.durationMs(), 0 /* deprecated */, + getDexoptedPackagesCount(packageDexoptResults), + getPackagesDependingOnBootClasspathCount(packageDexoptResults), + packageDexoptResults.size()); + } + + @NonNull + private static List<DexoptResult.PackageDexoptResult> getFilteredPackageResults( + @NonNull BackgroundDexoptJob.CompletedResult completedResult) { + return completedResult.dexoptResult() + .getPackageDexoptResults() + .stream() + .filter(packageResult + -> packageResult.getDexContainerFileDexoptResults().stream().anyMatch( + fileResult + -> (fileResult.getExtraStatus() + & DexoptResult.EXTRA_SKIPPED_NO_DEX_CODE) + == 0)) + .collect(Collectors.toList()); + } + + private static int getStatusForStats( + @NonNull BackgroundDexoptJob.CompletedResult result, Optional<Integer> stopReason) { + if (result.dexoptResult().getFinalStatus() == DexoptResult.DEXOPT_CANCELLED) { + if (stopReason.isPresent()) { + return ArtStatsLog + .BACKGROUND_DEXOPT_JOB_ENDED__STATUS__STATUS_ABORT_BY_CANCELLATION; + } else { + return ArtStatsLog.BACKGROUND_DEXOPT_JOB_ENDED__STATUS__STATUS_ABORT_BY_API; + } + } + + boolean isSkippedDueToStorageLow = + result.dexoptResult() + .getPackageDexoptResults() + .stream() + .flatMap(packageResult + -> packageResult.getDexContainerFileDexoptResults().stream()) + .anyMatch(fileResult + -> (fileResult.getExtraStatus() + & DexoptResult.EXTRA_SKIPPED_STORAGE_LOW) + != 0); + if (isSkippedDueToStorageLow) { + return ArtStatsLog.BACKGROUND_DEXOPT_JOB_ENDED__STATUS__STATUS_ABORT_NO_SPACE_LEFT; + } + + return ArtStatsLog.BACKGROUND_DEXOPT_JOB_ENDED__STATUS__STATUS_JOB_FINISHED; + } + + private static int getDexoptedPackagesCount( + @NonNull List<DexoptResult.PackageDexoptResult> packageResults) { + return (int) packageResults.stream() + .filter(result -> result.getStatus() == DexoptResult.DEXOPT_PERFORMED) + .count(); + } + + private static int getPackagesDependingOnBootClasspathCount( + @NonNull List<DexoptResult.PackageDexoptResult> packageResults) { + return (int) packageResults.stream() + .map(DexoptResult.PackageDexoptResult::getDexContainerFileDexoptResults) + .filter(BackgroundDexoptJobStatsReporter::isDependentOnBootClasspath) + .count(); + } + + private static boolean isDependentOnBootClasspath( + @NonNull List<DexoptResult.DexContainerFileDexoptResult> filesResults) { + return filesResults.stream() + .map(DexoptResult.DexContainerFileDexoptResult::getActualCompilerFilter) + .anyMatch(DexFile::isOptimizedCompilerFilter); + } +} diff --git a/libartservice/service/java/com/android/server/art/Dexopter.java b/libartservice/service/java/com/android/server/art/Dexopter.java index 446d9483b6..72fd22effc 100644 --- a/libartservice/service/java/com/android/server/art/Dexopter.java +++ b/libartservice/service/java/com/android/server/art/Dexopter.java @@ -165,7 +165,7 @@ public abstract class Dexopter<DexInfoType extends DetailedDexInfo> { long cpuTimeMs = 0; long sizeBytes = 0; long sizeBeforeBytes = 0; - boolean isSkippedDueToStorageLow = false; + @DexoptResult.DexoptResultExtraStatus int extraStatus = 0; try { var target = DexoptTarget.<DexInfoType>builder() .setDexInfo(dexInfo) @@ -182,6 +182,10 @@ public abstract class Dexopter<DexInfoType extends DetailedDexInfo> { GetDexoptNeededResult getDexoptNeededResult = getDexoptNeeded(target, options); + if (!getDexoptNeededResult.hasDexCode) { + extraStatus |= DexoptResult.EXTRA_SKIPPED_NO_DEX_CODE; + } + if (!getDexoptNeededResult.isDexoptNeeded) { continue; } @@ -196,7 +200,7 @@ public abstract class Dexopter<DexInfoType extends DetailedDexInfo> { && mInjector.getStorageManager().getAllocatableBytes( mPkg.getStorageUuid()) <= 0) { - isSkippedDueToStorageLow = true; + extraStatus |= DexoptResult.EXTRA_SKIPPED_STORAGE_LOW; continue; } } catch (IOException e) { @@ -239,7 +243,7 @@ public abstract class Dexopter<DexInfoType extends DetailedDexInfo> { } finally { var result = DexContainerFileDexoptResult.create(dexInfo.dexPath(), abi.isPrimaryAbi(), abi.name(), compilerFilter, status, wallTimeMs, - cpuTimeMs, sizeBytes, sizeBeforeBytes, isSkippedDueToStorageLow); + cpuTimeMs, sizeBytes, sizeBeforeBytes, extraStatus); Log.i(TAG, String.format("Dexopt result: [packageName = %s] %s", mPkgState.getPackageName(), result)); diff --git a/libartservice/service/java/com/android/server/art/model/DexoptResult.java b/libartservice/service/java/com/android/server/art/model/DexoptResult.java index 79f9b5f80a..b678a77aa5 100644 --- a/libartservice/service/java/com/android/server/art/model/DexoptResult.java +++ b/libartservice/service/java/com/android/server/art/model/DexoptResult.java @@ -28,6 +28,7 @@ import com.google.auto.value.AutoValue; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; +import java.util.ArrayList; import java.util.List; /** @hide */ @@ -59,6 +60,22 @@ public abstract class DexoptResult { @Retention(RetentionPolicy.SOURCE) public @interface DexoptResultStatus {} + // Possible values of {@link #DexoptResultExtraStatus}. + /** @hide */ + public static final int EXTRA_SKIPPED_STORAGE_LOW = 1 << 0; + /** @hide */ + public static final int EXTRA_SKIPPED_NO_DEX_CODE = 1 << 1; + + /** @hide */ + // clang-format off + @IntDef(flag = true, prefix = {"EXTRA_"}, value = { + EXTRA_SKIPPED_STORAGE_LOW, + EXTRA_SKIPPED_NO_DEX_CODE, + }) + // clang-format on + @Retention(RetentionPolicy.SOURCE) + public @interface DexoptResultExtraStatus {} + /** @hide */ protected DexoptResult() {} @@ -125,6 +142,19 @@ public abstract class DexoptResult { throw new IllegalArgumentException("Unknown dexopt status " + status); } + /** @hide */ + @NonNull + public static String dexoptResultExtraStatusToString(@DexoptResultExtraStatus int extraStatus) { + var strs = new ArrayList<String>(); + if ((extraStatus & DexoptResult.EXTRA_SKIPPED_STORAGE_LOW) != 0) { + strs.add("EXTRA_SKIPPED_STORAGE_LOW"); + } + if ((extraStatus & DexoptResult.EXTRA_SKIPPED_NO_DEX_CODE) != 0) { + strs.add("EXTRA_SKIPPED_NO_DEX_CODE"); + } + return String.join(", ", strs); + } + /** * Describes the result of a package. * @@ -192,10 +222,10 @@ public abstract class DexoptResult { boolean isPrimaryAbi, @NonNull String abi, @NonNull String compilerFilter, @DexoptResultStatus int status, long dex2oatWallTimeMillis, long dex2oatCpuTimeMillis, long sizeBytes, long sizeBeforeBytes, - boolean isSkippedDueToStorageLow) { + @DexoptResultExtraStatus int extraStatus) { return new AutoValue_DexoptResult_DexContainerFileDexoptResult(dexContainerFile, isPrimaryAbi, abi, compilerFilter, status, dex2oatWallTimeMillis, - dex2oatCpuTimeMillis, sizeBytes, sizeBeforeBytes, isSkippedDueToStorageLow); + dex2oatCpuTimeMillis, sizeBytes, sizeBeforeBytes, extraStatus); } /** The absolute path to the dex container file. */ @@ -250,7 +280,7 @@ public abstract class DexoptResult { public abstract long getSizeBeforeBytes(); /** @hide */ - public abstract boolean isSkippedDueToStorageLow(); + public abstract @DexoptResultExtraStatus int getExtraStatus(); @Override @NonNull @@ -264,11 +294,13 @@ public abstract class DexoptResult { + "dex2oatWallTimeMillis=%d, " + "dex2oatCpuTimeMillis=%d, " + "sizeBytes=%d, " - + "sizeBeforeBytes=%d}", + + "sizeBeforeBytes=%d, " + + "extraStatus=[%s]}", getDexContainerFile(), isPrimaryAbi(), getAbi(), getActualCompilerFilter(), DexoptResult.dexoptResultStatusToString(getStatus()), getDex2oatWallTimeMillis(), getDex2oatCpuTimeMillis(), getSizeBytes(), - getSizeBeforeBytes()); + getSizeBeforeBytes(), + DexoptResult.dexoptResultExtraStatusToString(getExtraStatus())); } } } diff --git a/libartservice/service/javatests/com/android/server/art/DexoptHelperTest.java b/libartservice/service/javatests/com/android/server/art/DexoptHelperTest.java index f08035dcee..21d89e65a3 100644 --- a/libartservice/service/javatests/com/android/server/art/DexoptHelperTest.java +++ b/libartservice/service/javatests/com/android/server/art/DexoptHelperTest.java @@ -822,11 +822,11 @@ public class DexoptHelperTest { return List.of(DexContainerFileDexoptResult.create(dexPath, true /* isPrimaryAbi */, "arm64-v8a", "verify", status1, 100 /* dex2oatWallTimeMillis */, 400 /* dex2oatCpuTimeMillis */, 0 /* sizeBytes */, - 0 /* sizeBeforeBytes */, false /* isSkippedDueToStorageLow */), + 0 /* sizeBeforeBytes */, 0 /* extraStatus */), DexContainerFileDexoptResult.create(dexPath, false /* isPrimaryAbi */, "armeabi-v7a", "verify", status2, 100 /* dex2oatWallTimeMillis */, 400 /* dex2oatCpuTimeMillis */, 0 /* sizeBytes */, 0 /* sizeBeforeBytes */, - false /* isSkippedDueToStorageLow */)); + 0 /* extraStatus */)); } private void checkPackageResult(DexoptResult result, int index, String packageName, diff --git a/libartservice/service/javatests/com/android/server/art/PrimaryDexopterParameterizedTest.java b/libartservice/service/javatests/com/android/server/art/PrimaryDexopterParameterizedTest.java index 46db32f808..fe9584797e 100644 --- a/libartservice/service/javatests/com/android/server/art/PrimaryDexopterParameterizedTest.java +++ b/libartservice/service/javatests/com/android/server/art/PrimaryDexopterParameterizedTest.java @@ -280,25 +280,23 @@ public class PrimaryDexopterParameterizedTest extends PrimaryDexopterTestBase { mParams.mExpectedCompilerFilter, DexoptResult.DEXOPT_PERFORMED, 100 /* dex2oatWallTimeMillis */, 400 /* dex2oatCpuTimeMillis */, 30000 /* sizeBytes */, 32000 /* sizeBeforeBytes */, - false /* isSkippedDueToStorageLow */), + 0 /* extraStatus */), DexContainerFileDexoptResult.create("/data/app/foo/base.apk", false /* isPrimaryAbi */, "armeabi-v7a", mParams.mExpectedCompilerFilter, DexoptResult.DEXOPT_FAILED, 0 /* dex2oatWallTimeMillis */, 0 /* dex2oatCpuTimeMillis */, - 0 /* sizeBytes */, 0 /* sizeBeforeBytes */, - false /* isSkippedDueToStorageLow */), + 0 /* sizeBytes */, 0 /* sizeBeforeBytes */, 0 /* extraStatus */), DexContainerFileDexoptResult.create("/data/app/foo/split_0.apk", true /* isPrimaryAbi */, "arm64-v8a", mParams.mExpectedCompilerFilter, DexoptResult.DEXOPT_SKIPPED, 0 /* dex2oatWallTimeMillis */, 0 /* dex2oatCpuTimeMillis */, - 0 /* sizeBytes */, 0 /* sizeBeforeBytes */, - false /* isSkippedDueToStorageLow */), + 0 /* sizeBytes */, 0 /* sizeBeforeBytes */, 0 /* extraStatus */), DexContainerFileDexoptResult.create("/data/app/foo/split_0.apk", false /* isPrimaryAbi */, "armeabi-v7a", mParams.mExpectedCompilerFilter, DexoptResult.DEXOPT_PERFORMED, 200 /* dex2oatWallTimeMillis */, 200 /* dex2oatCpuTimeMillis */, 10000 /* sizeBytes */, 0 /* sizeBeforeBytes */, - false /* isSkippedDueToStorageLow */)); + 0 /* extraStatus */)); // Verify that there are no more calls than the ones above. verify(mArtd, times(3)) diff --git a/libartservice/service/javatests/com/android/server/art/PrimaryDexopterTest.java b/libartservice/service/javatests/com/android/server/art/PrimaryDexopterTest.java index f02ebf0603..554b8c681c 100644 --- a/libartservice/service/javatests/com/android/server/art/PrimaryDexopterTest.java +++ b/libartservice/service/javatests/com/android/server/art/PrimaryDexopterTest.java @@ -609,19 +609,53 @@ public class PrimaryDexopterTest extends PrimaryDexopterTestBase { List<DexContainerFileDexoptResult> results = mPrimaryDexopter.dexopt(); assertThat(results.get(0).getStatus()).isEqualTo(DexoptResult.DEXOPT_PERFORMED); - assertThat(results.get(0).isSkippedDueToStorageLow()).isFalse(); + assertThat(results.get(0).getExtraStatus() & DexoptResult.EXTRA_SKIPPED_STORAGE_LOW) + .isEqualTo(0); assertThat(results.get(1).getStatus()).isEqualTo(DexoptResult.DEXOPT_SKIPPED); - assertThat(results.get(1).isSkippedDueToStorageLow()).isTrue(); + assertThat(results.get(1).getExtraStatus() & DexoptResult.EXTRA_SKIPPED_STORAGE_LOW) + .isNotEqualTo(0); assertThat(results.get(2).getStatus()).isEqualTo(DexoptResult.DEXOPT_SKIPPED); - assertThat(results.get(2).isSkippedDueToStorageLow()).isTrue(); + assertThat(results.get(2).getExtraStatus() & DexoptResult.EXTRA_SKIPPED_STORAGE_LOW) + .isNotEqualTo(0); assertThat(results.get(3).getStatus()).isEqualTo(DexoptResult.DEXOPT_PERFORMED); - assertThat(results.get(3).isSkippedDueToStorageLow()).isFalse(); + assertThat(results.get(3).getExtraStatus() & DexoptResult.EXTRA_SKIPPED_STORAGE_LOW) + .isEqualTo(0); verify(mArtd, times(2)) .dexopt(any(), any(), any(), any(), any(), any(), any(), any(), anyInt(), any(), any()); } + @Test + public void testDexoptDexStatus() throws Exception { + lenient() + .when(mArtd.getDexoptNeeded(any(), any(), any(), any(), anyInt())) + .thenReturn(dexoptIsNotNeeded(false /* hasDexCode */), + dexoptIsNotNeeded(false /* hasDexCode */), + dexoptIsNotNeeded(true /* hasDexCode */), dexoptIsNeeded()); + + mDexoptParams = new DexoptParams.Builder("install") + .setCompilerFilter("speed-profile") + .setFlags(ArtFlags.FLAG_FOR_PRIMARY_DEX) + .build(); + mPrimaryDexopter = + new PrimaryDexopter(mInjector, mPkgState, mPkg, mDexoptParams, mCancellationSignal); + + List<DexContainerFileDexoptResult> results = mPrimaryDexopter.dexopt(); + assertThat(results.get(0).getStatus()).isEqualTo(DexoptResult.DEXOPT_SKIPPED); + assertThat(results.get(0).getExtraStatus() & DexoptResult.EXTRA_SKIPPED_NO_DEX_CODE) + .isNotEqualTo(0); + assertThat(results.get(1).getStatus()).isEqualTo(DexoptResult.DEXOPT_SKIPPED); + assertThat(results.get(1).getExtraStatus() & DexoptResult.EXTRA_SKIPPED_NO_DEX_CODE) + .isNotEqualTo(0); + assertThat(results.get(2).getStatus()).isEqualTo(DexoptResult.DEXOPT_SKIPPED); + assertThat(results.get(2).getExtraStatus() & DexoptResult.EXTRA_SKIPPED_NO_DEX_CODE) + .isEqualTo(0); + assertThat(results.get(3).getStatus()).isEqualTo(DexoptResult.DEXOPT_PERFORMED); + assertThat(results.get(3).getExtraStatus() & DexoptResult.EXTRA_SKIPPED_NO_DEX_CODE) + .isEqualTo(0); + } + private void checkDexoptWithProfile(IArtd artd, String dexPath, String isa, ProfilePath profile, boolean isOtherReadable, boolean generateAppImage) throws Exception { artd.dexopt(argThat(artifacts diff --git a/libartservice/service/javatests/com/android/server/art/PrimaryDexopterTestBase.java b/libartservice/service/javatests/com/android/server/art/PrimaryDexopterTestBase.java index 81fe511ed5..df1527e2b7 100644 --- a/libartservice/service/javatests/com/android/server/art/PrimaryDexopterTestBase.java +++ b/libartservice/service/javatests/com/android/server/art/PrimaryDexopterTestBase.java @@ -169,8 +169,13 @@ public class PrimaryDexopterTestBase { } protected GetDexoptNeededResult dexoptIsNotNeeded() { + return dexoptIsNotNeeded(true /* hasDexCode */); + } + + protected GetDexoptNeededResult dexoptIsNotNeeded(boolean hasDexCode) { var result = new GetDexoptNeededResult(); result.isDexoptNeeded = false; + result.hasDexCode = hasDexCode; return result; } @@ -178,13 +183,14 @@ public class PrimaryDexopterTestBase { return dexoptIsNeeded(ArtifactsLocation.NONE_OR_ERROR); } - protected GetDexoptNeededResult dexoptIsNeeded(@ArtifactsLocation byte location) { + protected GetDexoptNeededResult dexoptIsNeeded(@ArtifactsLocation int location) { var result = new GetDexoptNeededResult(); result.isDexoptNeeded = true; result.artifactsLocation = location; if (location != ArtifactsLocation.NONE_OR_ERROR) { result.isVdexUsable = true; } + result.hasDexCode = true; return result; } diff --git a/libartservice/service/javatests/com/android/server/art/SecondaryDexopterTest.java b/libartservice/service/javatests/com/android/server/art/SecondaryDexopterTest.java index 5d8661fa05..35e256ac3c 100644 --- a/libartservice/service/javatests/com/android/server/art/SecondaryDexopterTest.java +++ b/libartservice/service/javatests/com/android/server/art/SecondaryDexopterTest.java @@ -167,23 +167,19 @@ public class SecondaryDexopterTest { DexContainerFileDexoptResult.create(DEX_1, true /* isPrimaryAbi */, "arm64-v8a", "speed-profile", DexoptResult.DEXOPT_PERFORMED, 0 /* dex2oatWallTimeMillis */, 0 /* dex2oatCpuTimeMillis */, - 0 /* sizeBytes */, 0 /* sizeBeforeBytes */, - false /* isSkippedDueToStorageLow */), + 0 /* sizeBytes */, 0 /* sizeBeforeBytes */, 0 /* extraStatus */), DexContainerFileDexoptResult.create(DEX_2, true /* isPrimaryAbi */, "arm64-v8a", "speed", DexoptResult.DEXOPT_PERFORMED, 0 /* dex2oatWallTimeMillis */, 0 /* dex2oatCpuTimeMillis */, - 0 /* sizeBytes */, 0 /* sizeBeforeBytes */, - false /* isSkippedDueToStorageLow */), + 0 /* sizeBytes */, 0 /* sizeBeforeBytes */, 0 /* extraStatus */), DexContainerFileDexoptResult.create(DEX_2, false /* isPrimaryAbi */, "armeabi-v7a", "speed", DexoptResult.DEXOPT_PERFORMED, 0 /* dex2oatWallTimeMillis */, 0 /* dex2oatCpuTimeMillis */, - 0 /* sizeBytes */, 0 /* sizeBeforeBytes */, - false /* isSkippedDueToStorageLow */), + 0 /* sizeBytes */, 0 /* sizeBeforeBytes */, 0 /* extraStatus */), DexContainerFileDexoptResult.create(DEX_3, true /* isPrimaryAbi */, "arm64-v8a", "verify", DexoptResult.DEXOPT_PERFORMED, 0 /* dex2oatWallTimeMillis */, 0 /* dex2oatCpuTimeMillis */, - 0 /* sizeBytes */, 0 /* sizeBeforeBytes */, - false /* isSkippedDueToStorageLow */)); + 0 /* sizeBytes */, 0 /* sizeBeforeBytes */, 0 /* extraStatus */)); // It should use profile for dex 1. @@ -304,6 +300,7 @@ public class SecondaryDexopterTest { result.isDexoptNeeded = true; result.artifactsLocation = ArtifactsLocation.NONE_OR_ERROR; result.isVdexUsable = false; + result.hasDexCode = true; return result; } diff --git a/odrefresh/odr_config.h b/odrefresh/odr_config.h index d86172175e..e87acc03f2 100644 --- a/odrefresh/odr_config.h +++ b/odrefresh/odr_config.h @@ -71,8 +71,8 @@ struct SystemPropertyConfig { // default value should not trigger re-compilation. This is to comply with the phenotype flag // requirement (go/platform-experiments-flags#pre-requisites). const android::base::NoDestructor<std::vector<SystemPropertyConfig>> kSystemProperties{ - {SystemPropertyConfig{.name = "persist.device_config.runtime_native_boot.enable_uffd_gc", - .default_value = ""}, + {SystemPropertyConfig{.name = "persist.device_config.runtime_native_boot.enable_uffd_gc_2", + .default_value = "false"}, SystemPropertyConfig{.name = kPhDisableCompactDex, .default_value = "false"}, SystemPropertyConfig{.name = kSystemPropertySystemServerCompilerFilterOverride, .default_value = ""}}}; diff --git a/odrefresh/odrefresh.cc b/odrefresh/odrefresh.cc index 7bbc922a8c..2fed8b7a2e 100644 --- a/odrefresh/odrefresh.cc +++ b/odrefresh/odrefresh.cc @@ -1385,7 +1385,7 @@ WARN_UNUSED BootImages OnDeviceRefresh::CheckBootClasspathArtifactsAreUpToDate( metrics.SetTrigger(data_result.GetTrigger()); } - if (boot_images_on_data.primary_boot_image) { + if (boot_images_on_system.primary_boot_image || boot_images_on_data.primary_boot_image) { if (data_result.IsBootImageMainlineExtensionOk()) { std::string error_msg; if (BootImageMainlineExtensionExist( diff --git a/runtime/art_field-inl.h b/runtime/art_field-inl.h index f6a99ac44e..e99642ca39 100644 --- a/runtime/art_field-inl.h +++ b/runtime/art_field-inl.h @@ -68,7 +68,7 @@ void ArtField::VisitArrayRoots(RootVisitorType& visitor, DCHECK_LE(start_boundary, end_boundary); DCHECK_NE(array->size(), 0u); ArtField* first_field = &array->At(0); - DCHECK_LE(static_cast<void*>(end_boundary), static_cast<void*>(first_field + array->size())); + end_boundary = std::min(end_boundary, reinterpret_cast<uint8_t*>(first_field + array->size())); static constexpr size_t kFieldSize = sizeof(ArtField); // Confirm the assumption that ArtField size is power of two. It's important // as we assume so below (RoundUp). diff --git a/runtime/art_method.h b/runtime/art_method.h index dce4066d0d..b5d6ee7365 100644 --- a/runtime/art_method.h +++ b/runtime/art_method.h @@ -865,6 +865,7 @@ class ArtMethod final { uint32_t access_flags = GetAccessFlags(); return !IsNative(access_flags) && !IsAbstract(access_flags) && + !IsDefaultConflicting(access_flags) && !IsRuntimeMethod() && !IsProxyMethod(); } @@ -1104,6 +1105,7 @@ class ArtMethod final { // - conflict method: ImtConflictTable, // - abstract/interface method: the single-implementation if any, // - proxy method: the original interface method or constructor, + // - default conflict method: null // - other methods: during AOT the code item offset, at runtime a pointer // to the code item. void* data_; diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc index ecb2fcf472..c4a3165e82 100644 --- a/runtime/class_linker.cc +++ b/runtime/class_linker.cc @@ -7999,6 +7999,7 @@ void ClassLinker::LinkMethodsHelper<kPointerSize>::ReallocMethods(ObjPtr<mirror: constexpr uint32_t kSetFlags = kAccDefault | kAccAbstract | kAccCopied; constexpr uint32_t kMaskFlags = ~(kAccSkipAccessChecks | kAccSingleImplementation); new_method.SetAccessFlags((access_flags | kSetFlags) & kMaskFlags); + new_method.SetDataPtrSize(nullptr, kPointerSize); DCHECK(new_method.IsDefaultConflicting()); DCHECK(!new_method.IsAbstract()); // The actual method might or might not be marked abstract since we just copied it from diff --git a/runtime/gc/collector/concurrent_copying.cc b/runtime/gc/collector/concurrent_copying.cc index 1f123aaff5..1c2f040805 100644 --- a/runtime/gc/collector/concurrent_copying.cc +++ b/runtime/gc/collector/concurrent_copying.cc @@ -478,9 +478,7 @@ class ConcurrentCopying::ThreadFlipVisitor : public Closure, public RootVisitor void Run(Thread* thread) override REQUIRES_SHARED(Locks::mutator_lock_) { // Note: self is not necessarily equal to thread since thread may be suspended. Thread* self = Thread::Current(); - CHECK(thread == self || - thread->IsSuspended() || - thread->GetState() == ThreadState::kWaitingPerformingGc) + CHECK(thread == self || thread->GetState() != ThreadState::kRunnable) << thread->GetState() << " thread " << thread << " self " << self; thread->SetIsGcMarkingAndUpdateEntrypoints(true); if (use_tlab_ && thread->HasTlab()) { diff --git a/runtime/gc/collector/immune_spaces.h b/runtime/gc/collector/immune_spaces.h index 5a8441a67a..950c35b27a 100644 --- a/runtime/gc/collector/immune_spaces.h +++ b/runtime/gc/collector/immune_spaces.h @@ -43,6 +43,7 @@ class ImmuneSpaces { public: ImmuneSpaces() {} void Reset(); + bool IsEmpty() const { return spaces_.empty(); } // Add a continuous space to the immune spaces set. void AddSpace(space::ContinuousSpace* space) REQUIRES(Locks::heap_bitmap_lock_); diff --git a/runtime/gc/collector/mark_compact.cc b/runtime/gc/collector/mark_compact.cc index bb34068fb1..5fd08b5e03 100644 --- a/runtime/gc/collector/mark_compact.cc +++ b/runtime/gc/collector/mark_compact.cc @@ -247,8 +247,9 @@ static bool GetCachedBoolProperty(const std::string& key, bool default_value) { static bool SysPropSaysUffdGc() { // The phenotype flag can change at time time after boot, but it shouldn't take effect until a // reboot. Therefore, we read the phenotype flag from the cache info, which is generated on boot. - return GetCachedBoolProperty("persist.device_config.runtime_native_boot.enable_uffd_gc", - GetBoolProperty("ro.dalvik.vm.enable_uffd_gc", false)); + return GetCachedBoolProperty("persist.device_config.runtime_native_boot.enable_uffd_gc_2", + false) || + GetBoolProperty("ro.dalvik.vm.enable_uffd_gc", false); } #else // Never called. @@ -560,27 +561,28 @@ void MarkCompact::AddLinearAllocSpaceData(uint8_t* begin, size_t len) { is_shared); } -void MarkCompact::BindAndResetBitmaps() { - // TODO: We need to hold heap_bitmap_lock_ only for populating immune_spaces. - // The card-table and mod-union-table processing can be done without it. So - // change the logic below. Note that the bitmap clearing would require the - // lock. +void MarkCompact::PrepareCardTableForMarking(bool clear_alloc_space_cards) { TimingLogger::ScopedTiming t(__FUNCTION__, GetTimings()); accounting::CardTable* const card_table = heap_->GetCardTable(); + // immune_spaces_ is emptied in InitializePhase() before marking starts. This + // function is invoked twice during marking. We only need to populate immune_spaces_ + // once per GC cycle. And when it's done (below), all the immune spaces are + // added to it. We can never have partially filled immune_spaces_. + bool update_immune_spaces = immune_spaces_.IsEmpty(); // Mark all of the spaces we never collect as immune. for (const auto& space : GetHeap()->GetContinuousSpaces()) { if (space->GetGcRetentionPolicy() == space::kGcRetentionPolicyNeverCollect || space->GetGcRetentionPolicy() == space::kGcRetentionPolicyFullCollect) { CHECK(space->IsZygoteSpace() || space->IsImageSpace()); - immune_spaces_.AddSpace(space); + if (update_immune_spaces) { + immune_spaces_.AddSpace(space); + } accounting::ModUnionTable* table = heap_->FindModUnionTableFromSpace(space); if (table != nullptr) { table->ProcessCards(); } else { - // Keep cards aged if we don't have a mod-union table since we may need + // Keep cards aged if we don't have a mod-union table since we need // to scan them in future GCs. This case is for app images. - // TODO: We could probably scan the objects right here to avoid doing - // another scan through the card-table. card_table->ModifyCardsAtomic( space->Begin(), space->End(), @@ -591,7 +593,7 @@ void MarkCompact::BindAndResetBitmaps() { }, /* card modified visitor */ VoidFunctor()); } - } else { + } else if (clear_alloc_space_cards) { CHECK(!space->IsZygoteSpace()); CHECK(!space->IsImageSpace()); // The card-table corresponding to bump-pointer and non-moving space can @@ -604,6 +606,16 @@ void MarkCompact::BindAndResetBitmaps() { non_moving_space_ = space; non_moving_space_bitmap_ = space->GetMarkBitmap(); } + } else { + card_table->ModifyCardsAtomic( + space->Begin(), + space->End(), + [](uint8_t card) { + return (card == gc::accounting::CardTable::kCardDirty) ? + gc::accounting::CardTable::kCardAged : + gc::accounting::CardTable::kCardClean; + }, + /* card modified visitor */ VoidFunctor()); } } } @@ -3807,28 +3819,6 @@ void MarkCompact::MarkReachableObjects() { ProcessMarkStack(); } -class MarkCompact::CardModifiedVisitor { - public: - explicit CardModifiedVisitor(MarkCompact* const mark_compact, - accounting::ContinuousSpaceBitmap* const bitmap, - accounting::CardTable* const card_table) - : visitor_(mark_compact), bitmap_(bitmap), card_table_(card_table) {} - - void operator()(uint8_t* card, - uint8_t expected_value, - uint8_t new_value ATTRIBUTE_UNUSED) const { - if (expected_value == accounting::CardTable::kCardDirty) { - uintptr_t start = reinterpret_cast<uintptr_t>(card_table_->AddrFromCard(card)); - bitmap_->VisitMarkedRange(start, start + accounting::CardTable::kCardSize, visitor_); - } - } - - private: - ScanObjectVisitor visitor_; - accounting::ContinuousSpaceBitmap* bitmap_; - accounting::CardTable* const card_table_; -}; - void MarkCompact::ScanDirtyObjects(bool paused, uint8_t minimum_age) { accounting::CardTable* card_table = heap_->GetCardTable(); for (const auto& space : heap_->GetContinuousSpaces()) { @@ -3848,58 +3838,8 @@ void MarkCompact::ScanDirtyObjects(bool paused, uint8_t minimum_age) { UNREACHABLE(); } TimingLogger::ScopedTiming t(name, GetTimings()); - ScanObjectVisitor visitor(this); - const bool is_immune_space = space->IsZygoteSpace() || space->IsImageSpace(); - if (paused) { - DCHECK_EQ(minimum_age, gc::accounting::CardTable::kCardDirty); - // We can clear the card-table for any non-immune space. - if (is_immune_space) { - card_table->Scan</*kClearCard*/false>(space->GetMarkBitmap(), - space->Begin(), - space->End(), - visitor, - minimum_age); - } else { - card_table->Scan</*kClearCard*/true>(space->GetMarkBitmap(), - space->Begin(), - space->End(), - visitor, - minimum_age); - } - } else { - DCHECK_EQ(minimum_age, gc::accounting::CardTable::kCardAged); - accounting::ModUnionTable* table = heap_->FindModUnionTableFromSpace(space); - if (table) { - table->ProcessCards(); - card_table->Scan</*kClearCard*/false>(space->GetMarkBitmap(), - space->Begin(), - space->End(), - visitor, - minimum_age); - } else { - CardModifiedVisitor card_modified_visitor(this, space->GetMarkBitmap(), card_table); - // For the alloc spaces we should age the dirty cards and clear the rest. - // For image and zygote-space without mod-union-table, age the dirty - // cards but keep the already aged cards unchanged. - // In either case, visit the objects on the cards that were changed from - // dirty to aged. - if (is_immune_space) { - card_table->ModifyCardsAtomic(space->Begin(), - space->End(), - [](uint8_t card) { - return (card == gc::accounting::CardTable::kCardClean) - ? card - : gc::accounting::CardTable::kCardAged; - }, - card_modified_visitor); - } else { - card_table->ModifyCardsAtomic(space->Begin(), - space->End(), - AgeCardVisitor(), - card_modified_visitor); - } - } - } + card_table->Scan</*kClearCard*/ false>( + space->GetMarkBitmap(), space->Begin(), space->End(), ScanObjectVisitor(this), minimum_age); } } @@ -3923,6 +3863,10 @@ void MarkCompact::MarkRoots(VisitRootFlags flags) { void MarkCompact::PreCleanCards() { TimingLogger::ScopedTiming t(__FUNCTION__, GetTimings()); CHECK(!Locks::mutator_lock_->IsExclusiveHeld(thread_running_gc_)); + // Age the card-table before thread stack scanning checkpoint in MarkRoots() + // as it ensures that there are no in-progress write barriers which started + // prior to aging the card-table. + PrepareCardTableForMarking(/*clear_alloc_space_cards*/ false); MarkRoots(static_cast<VisitRootFlags>(kVisitRootFlagClearRootLog | kVisitRootFlagNewRoots)); RecursiveMarkDirtyObjects(/*paused*/ false, accounting::CardTable::kCardDirty - 1); } @@ -3943,7 +3887,7 @@ void MarkCompact::MarkingPhase() { TimingLogger::ScopedTiming t(__FUNCTION__, GetTimings()); DCHECK_EQ(thread_running_gc_, Thread::Current()); WriterMutexLock mu(thread_running_gc_, *Locks::heap_bitmap_lock_); - BindAndResetBitmaps(); + PrepareCardTableForMarking(/*clear_alloc_space_cards*/ true); MarkZygoteLargeObjects(); MarkRoots( static_cast<VisitRootFlags>(kVisitRootFlagAllRoots | kVisitRootFlagStartLoggingNewRoots)); diff --git a/runtime/gc/collector/mark_compact.h b/runtime/gc/collector/mark_compact.h index d73f40d436..4e9780c970 100644 --- a/runtime/gc/collector/mark_compact.h +++ b/runtime/gc/collector/mark_compact.h @@ -290,10 +290,10 @@ class MarkCompact final : public GarbageCollector { // GC cycle. ALWAYS_INLINE mirror::Object* PostCompactBlackObjAddr(mirror::Object* old_ref) const REQUIRES_SHARED(Locks::mutator_lock_); - // Identify immune spaces and reset card-table, mod-union-table, and mark - // bitmaps. - void BindAndResetBitmaps() REQUIRES_SHARED(Locks::mutator_lock_) - REQUIRES(Locks::heap_bitmap_lock_); + // Clears (for alloc spaces in the beginning of marking phase) or ages the + // card table. Also, identifies immune spaces and mark bitmap. + void PrepareCardTableForMarking(bool clear_alloc_space_cards) + REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(Locks::heap_bitmap_lock_); // Perform one last round of marking, identifying roots from dirty cards // during a stop-the-world (STW) pause. @@ -767,8 +767,8 @@ class MarkCompact final : public GarbageCollector { class VerifyRootMarkedVisitor; class ScanObjectVisitor; class CheckpointMarkThreadRoots; - template<size_t kBufferSize> class ThreadRootsVisitor; - class CardModifiedVisitor; + template <size_t kBufferSize> + class ThreadRootsVisitor; class RefFieldsVisitor; template <bool kCheckBegin, bool kCheckEnd> class RefsUpdateVisitor; class ArenaPoolPageUpdater; diff --git a/runtime/oat_file_assistant.cc b/runtime/oat_file_assistant.cc index c735afa5f8..7191ec23e2 100644 --- a/runtime/oat_file_assistant.cc +++ b/runtime/oat_file_assistant.cc @@ -654,11 +654,7 @@ bool OatFileAssistant::DexLocationToOdexFilename(const std::string& location, // Get the base part of the file without the extension. std::string file = location.substr(pos + 1); pos = file.rfind('.'); - if (pos == std::string::npos) { - *error_msg = "Dex location " + location + " has no extension."; - return false; - } - std::string base = file.substr(0, pos); + std::string base = pos != std::string::npos ? file.substr(0, pos) : file; *odex_filename = dir + "/" + base + ".odex"; return true; @@ -1177,12 +1173,21 @@ bool OatFileAssistant::OatFileInfo::ShouldRecompileForFilter(CompilerFilter::Fil return true; } + // Don't regress the compiler filter for the triggers handled below. + if (CompilerFilter::IsBetter(current, target)) { + VLOG(oat) << "Should not recompile: current filter is better"; + return false; + } + if (dexopt_trigger.primaryBootImageBecomesUsable && - CompilerFilter::DependsOnImageChecksum(current)) { + CompilerFilter::IsAotCompilationEnabled(current)) { // If the oat file has been compiled without an image, and the runtime is // now running with an image loaded from disk, return that we need to // re-compile. The recompilation will generate a better oat file, and with an app // image for profile guided compilation. + // However, don't recompile for "verify". Although verification depends on the boot image, the + // penalty of being verified without a boot image is low. Consider the case where a dex file + // is verified by "ab-ota", we don't want it to be re-verified by "boot-after-ota". const char* oat_boot_class_path_checksums = file->GetOatHeader().GetStoreValueByKey(OatHeader::kBootClassPathChecksumsKey); if (oat_boot_class_path_checksums != nullptr && diff --git a/runtime/oat_file_assistant_test.cc b/runtime/oat_file_assistant_test.cc index 56d4c70920..bbaced5b61 100644 --- a/runtime/oat_file_assistant_test.cc +++ b/runtime/oat_file_assistant_test.cc @@ -1643,17 +1643,21 @@ TEST(OatFileAssistantUtilsTest, DexLocationToOdexFilename) { std::string odex_file; EXPECT_TRUE(OatFileAssistant::DexLocationToOdexFilename( - "/foo/bar/baz.jar", InstructionSet::kArm, &odex_file, &error_msg)) << error_msg; + "/foo/bar/baz.jar", InstructionSet::kArm, &odex_file, &error_msg)) + << error_msg; EXPECT_EQ("/foo/bar/oat/arm/baz.odex", odex_file); EXPECT_TRUE(OatFileAssistant::DexLocationToOdexFilename( - "/foo/bar/baz.funnyext", InstructionSet::kArm, &odex_file, &error_msg)) << error_msg; + "/foo/bar/baz.funnyext", InstructionSet::kArm, &odex_file, &error_msg)) + << error_msg; EXPECT_EQ("/foo/bar/oat/arm/baz.odex", odex_file); EXPECT_FALSE(OatFileAssistant::DexLocationToOdexFilename( - "nopath.jar", InstructionSet::kArm, &odex_file, &error_msg)); - EXPECT_FALSE(OatFileAssistant::DexLocationToOdexFilename( - "/foo/bar/baz_noext", InstructionSet::kArm, &odex_file, &error_msg)); + "nopath.jar", InstructionSet::kArm, &odex_file, &error_msg)); + + EXPECT_TRUE(OatFileAssistant::DexLocationToOdexFilename( + "/foo/bar/baz_noext", InstructionSet::kArm, &odex_file, &error_msg)); + EXPECT_EQ("/foo/bar/oat/arm/baz_noext.odex", odex_file); } // Verify the dexopt status values from dalvik.system.DexFile @@ -2106,6 +2110,152 @@ TEST_P(OatFileAssistantTest, VdexNoDex) { &oat_file_assistant, "unknown", "unknown", "io-error-no-apk"); } +// Case: We have a VDEX file, generated without a boot image, and we now have a boot image. +// Expect: Dexopt only if the target compiler filter >= "speed-profile". +TEST_P(OatFileAssistantTest, ShouldRecompileForImageFromVdex) { + std::string dex_location = GetScratchDir() + "/TestDex.jar"; + std::string odex_location = GetOdexDir() + "/TestDex.odex"; + std::string vdex_location = GetOdexDir() + "/TestDex.vdex"; + Copy(GetMultiDexSrc1(), dex_location); + + // Compile without a boot image. + GenerateOdexForTest(dex_location, + odex_location, + CompilerFilter::kVerify, + "install", + {"--boot-image=/nonx/boot.art"}); + + // Delete the odex file and only keep the vdex. + ASSERT_EQ(0, unlink(odex_location.c_str())); + + auto scoped_maybe_without_runtime = ScopedMaybeWithoutRuntime(); + + OatFileAssistant oat_file_assistant = CreateOatFileAssistant(dex_location.c_str()); + + VerifyGetDexOptNeeded(&oat_file_assistant, + CompilerFilter::kSpeed, + default_trigger_, + /*expected_dexopt_needed=*/true, + /*expected_is_vdex_usable=*/true, + /*expected_location=*/OatFileAssistant::kLocationOdex); + EXPECT_EQ(-OatFileAssistant::kDex2OatForFilter, + oat_file_assistant.GetDexOptNeeded(CompilerFilter::kSpeed)); + + VerifyGetDexOptNeeded(&oat_file_assistant, + CompilerFilter::kSpeedProfile, + default_trigger_, + /*expected_dexopt_needed=*/true, + /*expected_is_vdex_usable=*/true, + /*expected_location=*/OatFileAssistant::kLocationOdex); + EXPECT_EQ(-OatFileAssistant::kDex2OatForFilter, + oat_file_assistant.GetDexOptNeeded(CompilerFilter::kSpeedProfile)); + + VerifyGetDexOptNeeded(&oat_file_assistant, + CompilerFilter::kVerify, + default_trigger_, + /*expected_dexopt_needed=*/false, + /*expected_is_vdex_usable=*/true, + /*expected_location=*/OatFileAssistant::kLocationOdex); + EXPECT_EQ(OatFileAssistant::kNoDexOptNeeded, + oat_file_assistant.GetDexOptNeeded(CompilerFilter::kVerify)); +} + +// Case: We have an ODEX file, generated without a boot image (filter: "verify"), and we now have a +// boot image. +// Expect: Dexopt only if the target compiler filter >= "speed-profile". +TEST_P(OatFileAssistantTest, ShouldRecompileForImageFromVerify) { + std::string dex_location = GetScratchDir() + "/TestDex.jar"; + std::string odex_location = GetOdexDir() + "/TestDex.odex"; + std::string vdex_location = GetOdexDir() + "/TestDex.vdex"; + Copy(GetMultiDexSrc1(), dex_location); + + // Compile without a boot image. + GenerateOdexForTest(dex_location, + odex_location, + CompilerFilter::kVerify, + "install", + {"--boot-image=/nonx/boot.art"}); + + auto scoped_maybe_without_runtime = ScopedMaybeWithoutRuntime(); + + OatFileAssistant oat_file_assistant = CreateOatFileAssistant(dex_location.c_str()); + + VerifyGetDexOptNeeded(&oat_file_assistant, + CompilerFilter::kSpeed, + default_trigger_, + /*expected_dexopt_needed=*/true, + /*expected_is_vdex_usable=*/true, + /*expected_location=*/OatFileAssistant::kLocationOdex); + EXPECT_EQ(-OatFileAssistant::kDex2OatForFilter, + oat_file_assistant.GetDexOptNeeded(CompilerFilter::kSpeed)); + + VerifyGetDexOptNeeded(&oat_file_assistant, + CompilerFilter::kSpeedProfile, + default_trigger_, + /*expected_dexopt_needed=*/true, + /*expected_is_vdex_usable=*/true, + /*expected_location=*/OatFileAssistant::kLocationOdex); + EXPECT_EQ(-OatFileAssistant::kDex2OatForFilter, + oat_file_assistant.GetDexOptNeeded(CompilerFilter::kSpeedProfile)); + + VerifyGetDexOptNeeded(&oat_file_assistant, + CompilerFilter::kVerify, + default_trigger_, + /*expected_dexopt_needed=*/false, + /*expected_is_vdex_usable=*/true, + /*expected_location=*/OatFileAssistant::kLocationOdex); + EXPECT_EQ(OatFileAssistant::kNoDexOptNeeded, + oat_file_assistant.GetDexOptNeeded(CompilerFilter::kVerify)); +} + +// Case: We have an ODEX file, generated without a boot image (filter: "speed-profile"), and we now +// have a boot image. +// Expect: Dexopt only if the target compiler filter >= "speed-profile". +TEST_P(OatFileAssistantTest, ShouldRecompileForImageFromSpeedProfile) { + std::string dex_location = GetScratchDir() + "/TestDex.jar"; + std::string odex_location = GetOdexDir() + "/TestDex.odex"; + std::string vdex_location = GetOdexDir() + "/TestDex.vdex"; + Copy(GetMultiDexSrc1(), dex_location); + + // Compile without a boot image. + GenerateOdexForTest(dex_location, + odex_location, + CompilerFilter::kSpeedProfile, + "install", + {"--boot-image=/nonx/boot.art"}); + + auto scoped_maybe_without_runtime = ScopedMaybeWithoutRuntime(); + + OatFileAssistant oat_file_assistant = CreateOatFileAssistant(dex_location.c_str()); + + VerifyGetDexOptNeeded(&oat_file_assistant, + CompilerFilter::kSpeed, + default_trigger_, + /*expected_dexopt_needed=*/true, + /*expected_is_vdex_usable=*/true, + /*expected_location=*/OatFileAssistant::kLocationOdex); + EXPECT_EQ(-OatFileAssistant::kDex2OatForFilter, + oat_file_assistant.GetDexOptNeeded(CompilerFilter::kSpeed)); + + VerifyGetDexOptNeeded(&oat_file_assistant, + CompilerFilter::kSpeedProfile, + default_trigger_, + /*expected_dexopt_needed=*/true, + /*expected_is_vdex_usable=*/true, + /*expected_location=*/OatFileAssistant::kLocationOdex); + EXPECT_EQ(-OatFileAssistant::kDex2OatForFilter, + oat_file_assistant.GetDexOptNeeded(CompilerFilter::kSpeedProfile)); + + VerifyGetDexOptNeeded(&oat_file_assistant, + CompilerFilter::kVerify, + default_trigger_, + /*expected_dexopt_needed=*/false, + /*expected_is_vdex_usable=*/true, + /*expected_location=*/OatFileAssistant::kLocationOdex); + EXPECT_EQ(OatFileAssistant::kNoDexOptNeeded, + oat_file_assistant.GetDexOptNeeded(CompilerFilter::kVerify)); +} + // Test that GetLocation of a dex file is the same whether the dex // filed is backed by an oat file or not. TEST_F(OatFileAssistantBaseTest, GetDexLocation) { diff --git a/runtime/oat_file_manager.cc b/runtime/oat_file_manager.cc index 63e005dfe1..29b4560252 100644 --- a/runtime/oat_file_manager.cc +++ b/runtime/oat_file_manager.cc @@ -16,15 +16,16 @@ #include "oat_file_manager.h" +#include <stdlib.h> +#include <sys/stat.h> + #include <memory> #include <queue> #include <vector> -#include <sys/stat.h> #include "android-base/file.h" #include "android-base/stringprintf.h" #include "android-base/strings.h" - #include "art_field-inl.h" #include "base/bit_vector-inl.h" #include "base/file_utils.h" @@ -69,6 +70,10 @@ static constexpr bool kEnableAppImage = true; // If true, we attempt to load an app image generated by the runtime. static const bool kEnableRuntimeAppImage = true; +#if defined(__ANDROID__) +static const char* kDisableAppImageKeyword = "_disable_art_image_"; +#endif + const OatFile* OatFileManager::RegisterOatFile(std::unique_ptr<const OatFile> oat_file, bool in_memory) { // Use class_linker vlog to match the log for dex file registration. @@ -171,7 +176,20 @@ std::vector<const OatFile*> OatFileManager::RegisterImageOatFiles( bool OatFileManager::ShouldLoadAppImage(const OatFile* source_oat_file) const { Runtime* const runtime = Runtime::Current(); - return kEnableAppImage && (!runtime->IsJavaDebuggableAtInit() || source_oat_file->IsDebuggable()); + if (!kEnableAppImage || (runtime->IsJavaDebuggableAtInit() && !source_oat_file->IsDebuggable())) { + return false; + } + +#if defined(__ANDROID__) + const char* process_name = getprogname(); + // Some processes would rather take the runtime impact in the interest of memory (b/292210260) + if (process_name != nullptr && strstr(process_name, kDisableAppImageKeyword) != nullptr) { + LOG(INFO) << "Skipping app image load for " << process_name; + return false; + } +#endif + + return true; } std::vector<std::unique_ptr<const DexFile>> OatFileManager::OpenDexFilesFromOat( diff --git a/runtime/runtime_image.cc b/runtime/runtime_image.cc index f41d4c97f4..fa475f552b 100644 --- a/runtime/runtime_image.cc +++ b/runtime/runtime_image.cc @@ -953,10 +953,11 @@ class RuntimeImageHelper { ? StubType::kJNIDlsymLookupCriticalTrampoline : StubType::kJNIDlsymLookupTrampoline; copy->SetEntryPointFromJni(header.GetOatAddress(stub_type)); - } else if (method->IsInvokable()) { - DCHECK(method->HasCodeItem()) << method->PrettyMethod(); - ptrdiff_t code_item_offset = reinterpret_cast<const uint8_t*>(method->GetCodeItem()) - - method->GetDexFile()->DataBegin(); + } else if (method->HasCodeItem()) { + const uint8_t* code_item = reinterpret_cast<const uint8_t*>(method->GetCodeItem()); + DCHECK_GE(code_item, method->GetDexFile()->DataBegin()); + uint32_t code_item_offset = dchecked_integral_cast<uint32_t>( + code_item - method->GetDexFile()->DataBegin());; copy->SetDataPtrSize( reinterpret_cast<const void*>(code_item_offset), kRuntimePointerSize); } diff --git a/runtime/stack.cc b/runtime/stack.cc index d7d5851130..bf844b48e4 100644 --- a/runtime/stack.cc +++ b/runtime/stack.cc @@ -77,7 +77,7 @@ StackVisitor::StackVisitor(Thread* thread, context_(context), check_suspended_(check_suspended) { if (check_suspended_) { - DCHECK(thread == Thread::Current() || thread->IsSuspended()) << *thread; + DCHECK(thread == Thread::Current() || thread->GetState() != ThreadState::kRunnable) << *thread; } } @@ -801,7 +801,7 @@ uint8_t* StackVisitor::GetShouldDeoptimizeFlagAddr() const REQUIRES_SHARED(Locks template <StackVisitor::CountTransitions kCount> void StackVisitor::WalkStack(bool include_transitions) { if (check_suspended_) { - DCHECK(thread_ == Thread::Current() || thread_->IsSuspended()); + DCHECK(thread_ == Thread::Current() || thread_->GetState() != ThreadState::kRunnable); } CHECK_EQ(cur_depth_, 0U); diff --git a/runtime/thread-inl.h b/runtime/thread-inl.h index e4b3f6420c..ce50471815 100644 --- a/runtime/thread-inl.h +++ b/runtime/thread-inl.h @@ -347,25 +347,12 @@ inline ThreadState Thread::TransitionFromSuspendedToRunnable() { DCHECK_EQ(GetSuspendCount(), 0); } else if (UNLIKELY(old_state_and_flags.IsFlagSet(ThreadFlag::kRunningFlipFunction)) || UNLIKELY(old_state_and_flags.IsFlagSet(ThreadFlag::kWaitingForFlipFunction))) { - // The thread should be suspended while another thread is running the flip function. - static_assert(static_cast<std::underlying_type_t<ThreadState>>(ThreadState::kRunnable) == 0u); - LOG(FATAL) << "Transitioning to Runnable while another thread is running the flip function," - // Note: Keeping unused flags. If they are set, it points to memory corruption. - << " flags=" << old_state_and_flags.WithState(ThreadState::kRunnable).GetValue() - << " state=" << old_state_and_flags.GetState(); + // It's possible that some thread runs this thread's flip-function in + // Thread::GetPeerFromOtherThread() even though it was runnable. + WaitForFlipFunction(this); } else { DCHECK(old_state_and_flags.IsFlagSet(ThreadFlag::kPendingFlipFunction)); - // CAS the value with a memory barrier. - // Do not set `ThreadFlag::kRunningFlipFunction` as no other thread can run - // the flip function for a thread that is not suspended. - StateAndFlags new_state_and_flags = old_state_and_flags.WithState(ThreadState::kRunnable) - .WithoutFlag(ThreadFlag::kPendingFlipFunction); - if (LIKELY(tls32_.state_and_flags.CompareAndSetWeakAcquire(old_state_and_flags.GetValue(), - new_state_and_flags.GetValue()))) { - // Mark the acquisition of a share of the mutator lock. - GetMutatorLock()->TransitionFromSuspendedToRunnable(this); - // Run the flip function. - RunFlipFunction(this, /*notify=*/ false); + if (EnsureFlipFunctionStarted(this, old_state_and_flags)) { break; } } @@ -438,7 +425,7 @@ inline void Thread::RevokeThreadLocalAllocationStack() { if (kIsDebugBuild) { // Note: self is not necessarily equal to this thread since thread may be suspended. Thread* self = Thread::Current(); - DCHECK(this == self || IsSuspended() || GetState() == ThreadState::kWaitingPerformingGc) + DCHECK(this == self || GetState() != ThreadState::kRunnable) << GetState() << " thread " << this << " self " << self; } tlsPtr_.thread_local_alloc_stack_end = nullptr; diff --git a/runtime/thread.cc b/runtime/thread.cc index 6b1934c86e..f7fad4af45 100644 --- a/runtime/thread.cc +++ b/runtime/thread.cc @@ -145,6 +145,9 @@ thread_local Thread* Thread::self_tls_ = nullptr; #endif static constexpr bool kVerifyImageObjectsMarked = kIsDebugBuild; +// Amount of time (in microseconds) that we sleep if another thread is running +// flip function of the thread that we are interested in. +static constexpr size_t kSuspendTimeDuringFlip = 5'000; // For implicit overflow checks we reserve an extra piece of memory at the bottom // of the stack (lowest memory). The higher portion of the memory @@ -1606,25 +1609,8 @@ void Thread::ClearSuspendBarrier(AtomicInteger* target) { } void Thread::RunCheckpointFunction() { - // If this thread is suspended and another thread is running the checkpoint on its behalf, - // we may have a pending flip function that we need to run for the sake of those checkpoints - // that need to walk the stack. We should not see the flip function flags when the thread - // is running the checkpoint on its own. - StateAndFlags state_and_flags = GetStateAndFlags(std::memory_order_relaxed); - if (UNLIKELY(state_and_flags.IsAnyOfFlagsSet(FlipFunctionFlags()))) { - DCHECK(IsSuspended()); - Thread* self = Thread::Current(); - DCHECK(self != this); - if (state_and_flags.IsFlagSet(ThreadFlag::kPendingFlipFunction)) { - EnsureFlipFunctionStarted(self); - state_and_flags = GetStateAndFlags(std::memory_order_relaxed); - DCHECK(!state_and_flags.IsFlagSet(ThreadFlag::kPendingFlipFunction)); - } - if (state_and_flags.IsFlagSet(ThreadFlag::kRunningFlipFunction)) { - WaitForFlipFunction(self); - } - } - + DCHECK_EQ(Thread::Current(), this); + CHECK(!GetStateAndFlags(std::memory_order_relaxed).IsAnyOfFlagsSet(FlipFunctionFlags())); // Grab the suspend_count lock, get the next checkpoint and update all the checkpoint fields. If // there are no more checkpoints we will also clear the kCheckpointRequest flag. Closure* checkpoint; @@ -1812,7 +1798,7 @@ bool Thread::RequestSynchronousCheckpoint(Closure* function, ThreadState suspend !self->GetStateAndFlags(std::memory_order_relaxed).IsAnyOfFlagsSet(FlipFunctionFlags())); EnsureFlipFunctionStarted(self); while (GetStateAndFlags(std::memory_order_acquire).IsAnyOfFlagsSet(FlipFunctionFlags())) { - sched_yield(); + usleep(kSuspendTimeDuringFlip); } function->Run(this); @@ -1824,12 +1810,8 @@ bool Thread::RequestSynchronousCheckpoint(Closure* function, ThreadState suspend DCHECK_NE(GetState(), ThreadState::kRunnable); bool updated = ModifySuspendCount(self, -1, nullptr, SuspendReason::kInternal); DCHECK(updated); - } - - { // Imitate ResumeAll, the thread may be waiting on Thread::resume_cond_ since we raised its // suspend count. Now the suspend_count_ is lowered so we must do the broadcast. - MutexLock mu2(self, *Locks::thread_suspend_count_lock_); Thread::resume_cond_->Broadcast(self); } @@ -1850,23 +1832,43 @@ void Thread::SetFlipFunction(Closure* function) { AtomicSetFlag(ThreadFlag::kPendingFlipFunction, std::memory_order_release); } -void Thread::EnsureFlipFunctionStarted(Thread* self) { - while (true) { - StateAndFlags old_state_and_flags = GetStateAndFlags(std::memory_order_relaxed); - if (!old_state_and_flags.IsFlagSet(ThreadFlag::kPendingFlipFunction)) { - return; - } +bool Thread::EnsureFlipFunctionStarted(Thread* self, StateAndFlags old_state_and_flags) { + bool become_runnable; + if (old_state_and_flags.GetValue() == 0) { + become_runnable = false; + old_state_and_flags = GetStateAndFlags(std::memory_order_relaxed); + } else { + become_runnable = true; + DCHECK(!old_state_and_flags.IsFlagSet(ThreadFlag::kSuspendRequest)); + } + + while (old_state_and_flags.IsFlagSet(ThreadFlag::kPendingFlipFunction)) { DCHECK(!old_state_and_flags.IsFlagSet(ThreadFlag::kRunningFlipFunction)); StateAndFlags new_state_and_flags = old_state_and_flags.WithFlag(ThreadFlag::kRunningFlipFunction) .WithoutFlag(ThreadFlag::kPendingFlipFunction); + if (become_runnable) { + DCHECK_EQ(self, this); + DCHECK_NE(self->GetState(), ThreadState::kRunnable); + new_state_and_flags = new_state_and_flags.WithState(ThreadState::kRunnable); + } if (tls32_.state_and_flags.CompareAndSetWeakAcquire(old_state_and_flags.GetValue(), new_state_and_flags.GetValue())) { + if (become_runnable) { + GetMutatorLock()->TransitionFromSuspendedToRunnable(this); + } + art::Locks::mutator_lock_->AssertSharedHeld(self); RunFlipFunction(self, /*notify=*/ true); DCHECK(!GetStateAndFlags(std::memory_order_relaxed).IsAnyOfFlagsSet(FlipFunctionFlags())); - return; + return true; + } else { + old_state_and_flags = GetStateAndFlags(std::memory_order_relaxed); + if (become_runnable && old_state_and_flags.IsFlagSet(ThreadFlag::kSuspendRequest)) { + break; + } } } + return false; } void Thread::RunFlipFunction(Thread* self, bool notify) { @@ -4733,16 +4735,14 @@ bool Thread::IsAotCompiler() { return Runtime::Current()->IsAotCompiler(); } -mirror::Object* Thread::GetPeerFromOtherThread() const { +mirror::Object* Thread::GetPeerFromOtherThread() { DCHECK(tlsPtr_.jpeer == nullptr); - mirror::Object* peer = tlsPtr_.opeer; - if (gUseReadBarrier && Current()->GetIsGcMarking()) { - // We may call Thread::Dump() in the middle of the CC thread flip and this thread's stack - // may have not been flipped yet and peer may be a from-space (stale) ref. So explicitly - // mark/forward it here. - peer = art::ReadBarrier::Mark(peer); - } - return peer; + // Ensure that opeer is not obsolete. + EnsureFlipFunctionStarted(Thread::Current()); + while (GetStateAndFlags(std::memory_order_acquire).IsAnyOfFlagsSet(FlipFunctionFlags())) { + usleep(kSuspendTimeDuringFlip); + } + return tlsPtr_.opeer; } void Thread::SetReadBarrierEntrypoints() { diff --git a/runtime/thread.h b/runtime/thread.h index 5350330daf..89216311d9 100644 --- a/runtime/thread.h +++ b/runtime/thread.h @@ -383,15 +383,8 @@ class Thread { // Set the flip function. This is done with all threads suspended, except for the calling thread. void SetFlipFunction(Closure* function); - // Ensure that thread flip function started running. If no other thread is executing - // it, the calling thread shall run the flip function and then notify other threads - // that have tried to do that concurrently. After this function returns, the - // `ThreadFlag::kPendingFlipFunction` is cleared but another thread may still - // run the flip function as indicated by the `ThreadFlag::kRunningFlipFunction`. - void EnsureFlipFunctionStarted(Thread* self) REQUIRES_SHARED(Locks::mutator_lock_); - // Wait for the flip function to complete if still running on another thread. - void WaitForFlipFunction(Thread* self) REQUIRES_SHARED(Locks::mutator_lock_); + void WaitForFlipFunction(Thread* self); gc::accounting::AtomicStack<mirror::Object>* GetThreadLocalMarkStack() { CHECK(gUseReadBarrier); @@ -524,10 +517,12 @@ class Thread { CHECK(tlsPtr_.jpeer == nullptr); return tlsPtr_.opeer; } - // GetPeer is not safe if called on another thread in the middle of the CC thread flip and + // GetPeer is not safe if called on another thread in the middle of the thread flip and // the thread's stack may have not been flipped yet and peer may be a from-space (stale) ref. - // This function will explicitly mark/forward it. - mirror::Object* GetPeerFromOtherThread() const REQUIRES_SHARED(Locks::mutator_lock_); + // This function will force a flip for the other thread if necessary. + // Since we hold a shared mutator lock, a new flip function cannot be concurrently + // installed + mirror::Object* GetPeerFromOtherThread() REQUIRES_SHARED(Locks::mutator_lock_); bool HasPeer() const { return tlsPtr_.jpeer != nullptr || tlsPtr_.opeer != nullptr; @@ -1740,6 +1735,19 @@ class Thread { // to do that concurrently. void RunFlipFunction(Thread* self, bool notify) REQUIRES_SHARED(Locks::mutator_lock_); + // Ensure that thread flip function started running. If no other thread is executing + // it, the calling thread shall run the flip function and then notify other threads + // that have tried to do that concurrently. After this function returns, the + // `ThreadFlag::kPendingFlipFunction` is cleared but another thread may still + // run the flip function as indicated by the `ThreadFlag::kRunningFlipFunction`. + // A non-zero 'old_state_and_flags' indicates that the thread should logically + // acquire mutator lock if we win the race to run the flip function, if a + // suspend request is not already set. A zero 'old_state_and_flags' indicates + // we already hold the mutator lock. + // Returns true if this call ran the flip function. + bool EnsureFlipFunctionStarted(Thread* self, StateAndFlags old_state_and_flags = StateAndFlags(0)) + TRY_ACQUIRE_SHARED(true, Locks::mutator_lock_); + static void ThreadExitCallback(void* arg); // Maximum number of suspend barriers. @@ -2170,8 +2178,7 @@ class Thread { friend class QuickExceptionHandler; // For dumping the stack. friend class ScopedThreadStateChange; friend class StubTest; // For accessing entrypoints. - friend class ThreadList; // For ~Thread and Destroy. - + friend class ThreadList; // For ~Thread, Destroy and EnsureFlipFunctionStarted. friend class EntrypointsOrderTest; // To test the order of tls entries. friend class JniCompilerTest; // For intercepting JNI entrypoint calls. diff --git a/runtime/thread_list.cc b/runtime/thread_list.cc index d98415f644..9fa55f991d 100644 --- a/runtime/thread_list.cc +++ b/runtime/thread_list.cc @@ -388,17 +388,39 @@ size_t ThreadList::RunCheckpoint(Closure* checkpoint_function, Closure* callback // Run the checkpoint on ourself while we wait for threads to suspend. checkpoint_function->Run(self); + bool repeat = true; // Run the checkpoint on the suspended threads. - for (const auto& thread : suspended_count_modified_threads) { - // We know for sure that the thread is suspended at this point. - DCHECK(thread->IsSuspended()); - checkpoint_function->Run(thread); - { - MutexLock mu2(self, *Locks::thread_suspend_count_lock_); - bool updated = thread->ModifySuspendCount(self, -1, nullptr, SuspendReason::kInternal); - DCHECK(updated); + while (repeat) { + repeat = false; + for (auto& thread : suspended_count_modified_threads) { + if (thread != nullptr) { + // We know for sure that the thread is suspended at this point. + DCHECK(thread->IsSuspended()); + // Make sure there is no pending flip function before running checkpoint + // on behalf of thread. + thread->EnsureFlipFunctionStarted(self); + if (thread->GetStateAndFlags(std::memory_order_acquire) + .IsAnyOfFlagsSet(Thread::FlipFunctionFlags())) { + // There is another thread running the flip function for 'thread'. + // Instead of waiting for it to complete, move to the next thread. + repeat = true; + continue; + } + checkpoint_function->Run(thread); + { + MutexLock mu2(self, *Locks::thread_suspend_count_lock_); + bool updated = thread->ModifySuspendCount(self, -1, nullptr, SuspendReason::kInternal); + DCHECK(updated); + } + // We are done with 'thread' so set it to nullptr so that next outer + // loop iteration, if any, skips 'thread'. + thread = nullptr; + } } } + DCHECK(std::all_of(suspended_count_modified_threads.cbegin(), + suspended_count_modified_threads.cend(), + [](Thread* thread) { return thread == nullptr; })); { // Imitate ResumeAll, threads may be waiting on Thread::resume_cond_ since we raised their diff --git a/runtime/trace.cc b/runtime/trace.cc index 429a5ae591..92f46c299c 100644 --- a/runtime/trace.cc +++ b/runtime/trace.cc @@ -91,10 +91,14 @@ double tsc_to_microsec_scaling_factor = -1.0; uint64_t GetTimestamp() { uint64_t t = 0; #if defined(__arm__) - // See Architecture Reference Manual ARMv7-A and ARMv7-R edition section B4.1.34 - // Q and R specify that they should be written to lower and upper halves of 64-bit value. - // See: https://llvm.org/docs/LangRef.html#asm-template-argument-modifiers - asm volatile("mrrc p15, 1, %Q0, %R0, c14" : "=r"(t)); + // On ARM 32 bit, we don't always have access to the timestamp counters from user space. There is + // no easy way to check if it is safe to read the timestamp counters. There is HWCAP_EVTSTRM which + // is set when generic timer is available but not necessarily from the user space. Kernel disables + // access to generic timer when there are known problems on the target CPUs. Sometimes access is + // disabled only for 32-bit processes even when 64-bit processes can accesses the timer from user + // space. These are not reflected in the HWCAP_EVTSTRM capability.So just fallback to + // clock_gettime on these processes. See b/289178149 for more discussion. + t = MicroTime(); #elif defined(__aarch64__) // See Arm Architecture Registers Armv8 section System Registers asm volatile("mrs %0, cntvct_el0" : "=r"(t)); @@ -173,11 +177,9 @@ void InitializeTimestampCounters() { } #if defined(__arm__) - double seconds_to_microseconds = 1000 * 1000; - uint64_t freq = 0; - // See Architecture Reference Manual ARMv7-A and ARMv7-R edition section B4.1.21 - asm volatile("mrc p15, 0, %0, c14, c0, 0" : "=r"(freq)); - tsc_to_microsec_scaling_factor = seconds_to_microseconds / static_cast<double>(freq); + // On ARM 32 bit, we don't always have access to the timestamp counters from + // user space. Seem comment in GetTimestamp for more details. + tsc_to_microsec_scaling_factor = 1.0; #elif defined(__aarch64__) double seconds_to_microseconds = 1000 * 1000; uint64_t freq = 0; diff --git a/test/845-data-image/src-art/IfaceWithSayHi.java b/test/845-data-image/src-art/IfaceWithSayHi.java new file mode 100644 index 0000000000..2fcbd4e131 --- /dev/null +++ b/test/845-data-image/src-art/IfaceWithSayHi.java @@ -0,0 +1,21 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * 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. + */ + +public interface IfaceWithSayHi { + public default String sayHi() { + return "Hi"; + } +} diff --git a/test/845-data-image/src-art/IfaceWithSayHiAtRuntime.java b/test/845-data-image/src-art/IfaceWithSayHiAtRuntime.java new file mode 100644 index 0000000000..23b93940bd --- /dev/null +++ b/test/845-data-image/src-art/IfaceWithSayHiAtRuntime.java @@ -0,0 +1,18 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * 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. + */ + +public interface IfaceWithSayHiAtRuntime { +} diff --git a/test/845-data-image/src-art/Main.java b/test/845-data-image/src-art/Main.java index e74a6d69a0..36cc9d0bca 100644 --- a/test/845-data-image/src-art/Main.java +++ b/test/845-data-image/src-art/Main.java @@ -108,6 +108,9 @@ interface Itf2 { class Itf2Impl implements Itf2 { } +class ClassWithDefaultConflict implements IfaceWithSayHi, IfaceWithSayHiAtRuntime { +} + public class Main implements Itf { static String myString = "MyString"; @@ -220,6 +223,7 @@ public class Main implements Itf { public static Itf2 itf2 = new Itf2Impl(); public static ClassWithStatics statics = new ClassWithStatics(); public static ClassWithStaticType staticType = new ClassWithStaticType(); + public static ClassWithDefaultConflict defaultConflict = new ClassWithDefaultConflict(); public static void runClassTests() { // Test Class.getName, app images expect all strings to have hash codes. diff --git a/test/845-data-image/src2/IfaceWithSayHiAtRuntime.java b/test/845-data-image/src2/IfaceWithSayHiAtRuntime.java new file mode 100644 index 0000000000..9c3a339462 --- /dev/null +++ b/test/845-data-image/src2/IfaceWithSayHiAtRuntime.java @@ -0,0 +1,21 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * 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. + */ + +public interface IfaceWithSayHiAtRuntime { + public default String sayHi() { + return "hello"; + } +} diff --git a/test/odsign/test-src/com/android/tests/odsign/DeviceState.java b/test/odsign/test-src/com/android/tests/odsign/DeviceState.java index 92958311f2..53cdaa6371 100644 --- a/test/odsign/test-src/com/android/tests/odsign/DeviceState.java +++ b/test/odsign/test-src/com/android/tests/odsign/DeviceState.java @@ -51,7 +51,7 @@ public class DeviceState { private Set<String> mTempFiles = new HashSet<>(); private Set<String> mMountPoints = new HashSet<>(); private Map<String, String> mMutatedProperties = new HashMap<>(); - private Set<String> mMutatedPhenotypeFlags = new HashSet<>(); + private Map<String, String> mMutatedPhenotypeFlags = new HashMap<>(); private Map<String, String> mDeletedFiles = new HashMap<>(); private boolean mHasArtifactsBackup = false; @@ -75,9 +75,16 @@ public class DeviceState { entry.getKey(), entry.getValue() != null ? entry.getValue() : ""); } - for (String flag : mMutatedPhenotypeFlags) { - mTestInfo.getDevice().executeShellV2Command(String.format( - "device_config delete '%s' '%s'", PHENOTYPE_FLAG_NAMESPACE, flag)); + for (var entry : mMutatedPhenotypeFlags.entrySet()) { + if (entry.getValue() != null) { + mTestInfo.getDevice().executeShellV2Command( + String.format("device_config put '%s' '%s' '%s'", PHENOTYPE_FLAG_NAMESPACE, + entry.getKey(), entry.getValue())); + } else { + mTestInfo.getDevice().executeShellV2Command( + String.format("device_config delete '%s' '%s'", PHENOTYPE_FLAG_NAMESPACE, + entry.getKey())); + } } if (!mMutatedPhenotypeFlags.isEmpty()) { @@ -174,13 +181,10 @@ public class DeviceState { /** Sets a phenotype flag. */ public void setPhenotypeFlag(String key, String value) throws Exception { - if (!mMutatedPhenotypeFlags.contains(key)) { - // Tests assume that phenotype flags are initially not set. Check if the assumption is - // true. - assertThat(mTestUtils.assertCommandSucceeds(String.format( - "device_config get '%s' '%s'", PHENOTYPE_FLAG_NAMESPACE, key))) - .isEqualTo("null"); - mMutatedPhenotypeFlags.add(key); + if (!mMutatedPhenotypeFlags.containsKey(key)) { + String output = mTestUtils.assertCommandSucceeds( + String.format("device_config get '%s' '%s'", PHENOTYPE_FLAG_NAMESPACE, key)); + mMutatedPhenotypeFlags.put(key, output.equals("null") ? null : output); } // Disable phenotype flag syncing. Potentially, we can set `set_sync_disabled_for_tests` to diff --git a/test/odsign/test-src/com/android/tests/odsign/OdrefreshFactoryHostTestBase.java b/test/odsign/test-src/com/android/tests/odsign/OdrefreshFactoryHostTestBase.java index 900e978c12..eaf2ec212c 100644 --- a/test/odsign/test-src/com/android/tests/odsign/OdrefreshFactoryHostTestBase.java +++ b/test/odsign/test-src/com/android/tests/odsign/OdrefreshFactoryHostTestBase.java @@ -112,6 +112,14 @@ abstract public class OdrefreshFactoryHostTestBase extends BaseHostJUnit4Test { mTestUtils.assertModifiedAfter(mTestUtils.getExpectedBootImageMainlineExtension(), timeMs); mTestUtils.assertModifiedAfter(mTestUtils.getSystemServerExpectedArtifacts(), timeMs); + // It should not recompile the boot image and system server after odrefresh run again + timeMs = mTestUtils.getCurrentTimeMs(); + mTestUtils.runOdrefresh(); + mTestUtils.assertNotModifiedAfter(Set.of(OdsignTestUtils.CACHE_INFO_FILE), timeMs); + mTestUtils.assertFilesNotExist(mTestUtils.getExpectedPrimaryBootImage()); + mTestUtils.assertNotModifiedAfter(mTestUtils.getExpectedBootImageMainlineExtension(), timeMs); + mTestUtils.assertNotModifiedAfter(mTestUtils.getSystemServerExpectedArtifacts(), timeMs); + // Generated artifacts should be loaded. mTestUtils.restartZygote(); mTestUtils.verifyZygotesLoadedBootImageMainlineExtension(); @@ -177,11 +185,17 @@ abstract public class OdrefreshFactoryHostTestBase extends BaseHostJUnit4Test { @Test public void verifyEnableUffdGcChangeTriggersCompilation() throws Exception { - mDeviceState.setPhenotypeFlag("enable_uffd_gc", "true"); + // Simulate that the flag value is initially empty. + mDeviceState.setPhenotypeFlag("enable_uffd_gc_2", null); long timeMs = mTestUtils.getCurrentTimeMs(); mTestUtils.runOdrefresh(); + mDeviceState.setPhenotypeFlag("enable_uffd_gc_2", "true"); + + timeMs = mTestUtils.getCurrentTimeMs(); + mTestUtils.runOdrefresh(); + // It should recompile everything. mTestUtils.assertModifiedAfter(Set.of(OdsignTestUtils.CACHE_INFO_FILE), timeMs); mTestUtils.assertModifiedAfter(mTestUtils.getExpectedPrimaryBootImage(), timeMs); @@ -199,7 +213,7 @@ abstract public class OdrefreshFactoryHostTestBase extends BaseHostJUnit4Test { mTestUtils.getExpectedBootImageMainlineExtension(), timeMs); mTestUtils.assertNotModifiedAfter(mTestUtils.getSystemServerExpectedArtifacts(), timeMs); - mDeviceState.setPhenotypeFlag("enable_uffd_gc", null); + mDeviceState.setPhenotypeFlag("enable_uffd_gc_2", null); mTestUtils.runOdrefresh(); diff --git a/test/odsign/test-src/com/android/tests/odsign/OdrefreshHostTest.java b/test/odsign/test-src/com/android/tests/odsign/OdrefreshHostTest.java index c5af9d84de..ae275d3b32 100644 --- a/test/odsign/test-src/com/android/tests/odsign/OdrefreshHostTest.java +++ b/test/odsign/test-src/com/android/tests/odsign/OdrefreshHostTest.java @@ -131,17 +131,24 @@ public class OdrefreshHostTest extends BaseHostJUnit4Test { @Test public void verifyEnableUffdGcChangeTriggersCompilation() throws Exception { - mDeviceState.setPhenotypeFlag("enable_uffd_gc", "false"); + // Simulate that the flag value is initially empty. + mDeviceState.setPhenotypeFlag("enable_uffd_gc_2", null); long timeMs = mTestUtils.getCurrentTimeMs(); mTestUtils.runOdrefresh(); - // Artifacts should be re-compiled. - mTestUtils.assertModifiedAfter(mTestUtils.getExpectedPrimaryBootImage(), timeMs); - mTestUtils.assertModifiedAfter(mTestUtils.getExpectedBootImageMainlineExtension(), timeMs); - mTestUtils.assertModifiedAfter(mTestUtils.getSystemServerExpectedArtifacts(), timeMs); + mDeviceState.setPhenotypeFlag("enable_uffd_gc_2", "false"); + + timeMs = mTestUtils.getCurrentTimeMs(); + mTestUtils.runOdrefresh(); + + // Artifacts should not be re-compiled. + mTestUtils.assertNotModifiedAfter(mTestUtils.getExpectedPrimaryBootImage(), timeMs); + mTestUtils.assertNotModifiedAfter( + mTestUtils.getExpectedBootImageMainlineExtension(), timeMs); + mTestUtils.assertNotModifiedAfter(mTestUtils.getSystemServerExpectedArtifacts(), timeMs); - mDeviceState.setPhenotypeFlag("enable_uffd_gc", "true"); + mDeviceState.setPhenotypeFlag("enable_uffd_gc_2", "true"); timeMs = mTestUtils.getCurrentTimeMs(); mTestUtils.runOdrefresh(); @@ -161,7 +168,7 @@ public class OdrefreshHostTest extends BaseHostJUnit4Test { mTestUtils.getExpectedBootImageMainlineExtension(), timeMs); mTestUtils.assertNotModifiedAfter(mTestUtils.getSystemServerExpectedArtifacts(), timeMs); - mDeviceState.setPhenotypeFlag("enable_uffd_gc", null); + mDeviceState.setPhenotypeFlag("enable_uffd_gc_2", null); timeMs = mTestUtils.getCurrentTimeMs(); mTestUtils.runOdrefresh(); |