Reland "ARM64: Enable implicit suspend checks after CCCR."
This reverts commit 2de08ed613903526279512c7f6114b52cbf5e993.
Reason for revert: Reland with app startup performance
impact mitigation. Replace `MADV_FREE` with `MADV_DONTNEED`
in `Thread::MadviseAwayAlternateSignalStack()`.
Some regressions are expected but this change should provide
a good trade-off overall. The benefits for memory use thanks
to reduced code size can be seen in b/263509016 that tracked
the system server RSS regression when we disabled implicit
suspend checks the last time.
The startup regression b/263319992 has been partially
mitigated by
https://android-review.googlesource.com/2375891
and is being partially mitigated by using the `MADV_FREE`,
see above. Based on some test runs, the `MADV_FREE` roughly
halves the regression we would incur with `MADV_DONTNEED`.
On the flip side, the `MADV_FREE` does not immediatelly
reclaim the committed memory of the alternate signal stacks,
so we may see regressions in some memory benchmarks. These
regressions would not be true regressions, however, because
that memory is indeed reclaimable under memory pressure, so
it should be considered as any other temporary allocation.
Some Golem micro-benchmarks show significant improvements on
Odroid-C2 while showing significant regressions on Raspberry
Pi 4 on the very same benchmark. The implicit suspend check
is a single LDR while the explicit check is LDR+TST+BNE and
it seems reasonable to expect better performance on in-order
CPUs due to fewer instructions being executed. However,
out-of-order CPUs may prefer the LDR for explicit checks as
it uses a "constant" address over the LDR for implicit check
which uses the previously loaded value as the the address
for the next check, presumably preventing scheduling these
LDRs closely together, for example in a tight loop.
Test: run-gtests.sh
Test: testrunner.py --target --64 --optimizing --jit
Bug: 38383823
Bug: 215310343
Change-Id: I274a5417293e6d926579ec97190085387137d7c2
diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index 1754a68..f205565 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -845,9 +845,7 @@
// Set the compilation target's implicit checks options.
switch (compiler_options_->GetInstructionSet()) {
case InstructionSet::kArm64:
- // TODO: Implicit suspend checks are currently disabled to facilitate search
- // for unrelated memory use regressions. Bug: 213757852.
- compiler_options_->implicit_suspend_checks_ = false;
+ compiler_options_->implicit_suspend_checks_ = true;
FALLTHROUGH_INTENDED;
case InstructionSet::kArm:
case InstructionSet::kThumb2:
diff --git a/runtime/oat.h b/runtime/oat.h
index 20d32cb..3b32e11 100644
--- a/runtime/oat.h
+++ b/runtime/oat.h
@@ -32,8 +32,8 @@
class PACKED(4) OatHeader {
public:
static constexpr std::array<uint8_t, 4> kOatMagic { { 'o', 'a', 't', '\n' } };
- // Last oat version changed reason: Add a new QuickEntryPoint for a String constructor.
- static constexpr std::array<uint8_t, 4> kOatVersion { { '2', '2', '9', '\0' } };
+ // Last oat version changed reason: ARM64: Enable implicit suspend checks; compiled code check.
+ static constexpr std::array<uint8_t, 4> kOatVersion { { '2', '3', '0', '\0' } };
static constexpr const char* kDex2OatCmdLineKey = "dex2oat-cmdline";
static constexpr const char* kDebuggableKey = "debuggable";
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index 8c0150e..27a1c58 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -1751,9 +1751,7 @@
// Change the implicit checks flags based on runtime architecture.
switch (kRuntimeISA) {
case InstructionSet::kArm64:
- // TODO: Implicit suspend checks are currently disabled to facilitate search
- // for unrelated memory use regressions. Bug: 213757852.
- implicit_suspend_checks_ = false;
+ implicit_suspend_checks_ = true;
FALLTHROUGH_INTENDED;
case InstructionSet::kArm:
case InstructionSet::kThumb2:
diff --git a/runtime/thread_android.cc b/runtime/thread_android.cc
index e8d01cc..9df1532 100644
--- a/runtime/thread_android.cc
+++ b/runtime/thread_android.cc
@@ -42,7 +42,7 @@
IsAligned<kPageSize>(old_ss.ss_sp) &&
IsAligned<kPageSize>(old_ss.ss_size)) {
CHECK_EQ(old_ss.ss_flags & SS_ONSTACK, 0);
- result = madvise(old_ss.ss_sp, old_ss.ss_size, MADV_DONTNEED);
+ result = madvise(old_ss.ss_sp, old_ss.ss_size, MADV_FREE);
CHECK_EQ(result, 0);
}
}
diff --git a/test/2233-checker-remove-loop-suspend-check/src/Main.java b/test/2233-checker-remove-loop-suspend-check/src/Main.java
index bea67e7..c56bd66 100644
--- a/test/2233-checker-remove-loop-suspend-check/src/Main.java
+++ b/test/2233-checker-remove-loop-suspend-check/src/Main.java
@@ -26,7 +26,6 @@
/// CHECK-NEXT: dex_pc:{{.*}}
/// CHECK: Goto loop:<<LoopId>>
/// CHECK-NEXT: b
- /// CHECK-NOT: SuspendCheckSlowPathARM64
public static void $noinline$testRemoveSuspendCheck(int[] a) {
for (int i = 0; i < ITERATIONS; i++) {
@@ -41,8 +40,6 @@
/// CHECK: SuspendCheck loop:<<LoopId:B\d+>>
/// CHECK: Goto loop:<<LoopId>>
/// CHECK-NEXT: ldr
- /// CHECK: SuspendCheckSlowPathARM64
- /// CHECK: SuspendCheckSlowPathARM64
public static void testRemoveSuspendCheckWithCall(int[] a) {
for (int i = 0; i < ITERATIONS; i++) {
@@ -58,7 +55,6 @@
/// CHECK: SuspendCheck loop:<<LoopId:B\d+>>
/// CHECK: Goto loop:<<LoopId>>
/// CHECK-NEXT: ldr
- /// CHECK: SuspendCheckSlowPathARM64
public static void testRemoveSuspendCheckAboveHeuristic(int[] a) {
for (int i = 0; i < ITERATIONS * 6; i++) {
@@ -73,7 +69,6 @@
/// CHECK: SuspendCheck loop:<<LoopId:B\d+>>
/// CHECK: Goto loop:<<LoopId>>
/// CHECK-NEXT: ldr
- /// CHECK: SuspendCheckSlowPathARM64
public static void testRemoveSuspendCheckUnknownCount(int[] a, int n) {
for (int i = 0; i < n; i++) {