Revert "Revert "Make it possible to enable native debugging through debug flags""

This reverts commit 3a98aae1b9b20bc78dc5e05d2e60cb1d0072db02.

* Add support for a new debug flag disabling the optimizations in
  the compiler and enable the generation of some additional debug
  info (--native-debuggable).
* Ignore the content of the oat files if force JIT is enabled so
  the runtime ignores the AOT-ed code what doesn't contain any
  debug info.

Time measurements on a Nexus 5 with running:
am start -n com.facebook.katana/com.facebook.katana.LoginActivity -W

Before change:             | AVG | DEV
--------------------------------------
ThisTime:  549 492 512 511 | 516 | 24
TotalTime: 549 492 512 511 | 516 | 24
WaitTime:  662 511 528 526 | 557 | 71

After change:              | AVG | DEV
--------------------------------------
ThisTime:  530 467 503 544 | 511 | 34
TotalTime: 530 467 503 544 | 511 | 34
WaitTime:  551 497 536 583 | 541 | 36

Based on the numbers the speed impact of the change is less then the
accuracy of the measurement and it is also negligible.

The minor speed improvement displayed in the measurements are just
the cause of the variance of the measurement and not caused by this
change.

Change-Id: Ibf7294bfd14f8632a0fdeb27a5b90bfe5037d085

diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 0631ebe..5278d1b 100644 (file)
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -2620,19 +2620,37 @@ const void* ClassLinker::GetQuickOatCodeFor(const DexFile& dex_file,
   return oat_class.GetOatMethod(oat_method_idx).GetQuickCode();
 }
 
-// Returns true if the method must run with interpreter, false otherwise.
-static bool NeedsInterpreter(ArtMethod* method, const void* quick_code)
-    SHARED_REQUIRES(Locks::mutator_lock_) {
+bool ClassLinker::ShouldUseInterpreterEntrypoint(ArtMethod* method, const void* quick_code) {
+  if (UNLIKELY(method->IsNative() || method->IsProxyMethod())) {
+    return false;
+  }
+
   if (quick_code == nullptr) {
-    // No code: need interpreter.
-    // May return true for native code, in the case of generic JNI
-    // DCHECK(!method->IsNative());
     return true;
   }
-  // If interpreter mode is enabled, every method (except native and proxy) must
-  // be run with interpreter.
-  return Runtime::Current()->GetInstrumentation()->InterpretOnly() &&
-         !method->IsNative() && !method->IsProxyMethod();
+
+  Runtime* runtime = Runtime::Current();
+  instrumentation::Instrumentation* instr = runtime->GetInstrumentation();
+  if (instr->InterpretOnly()) {
+    return true;
+  }
+
+  if (runtime->GetClassLinker()->IsQuickToInterpreterBridge(quick_code)) {
+    // Doing this check avoids doing compiled/interpreter transitions.
+    return true;
+  }
+
+  if (Dbg::IsForcedInterpreterNeededForCalling(Thread::Current(), method)) {
+    // Force the use of interpreter when it is required by the debugger.
+    return true;
+  }
+
+  if (runtime->UseJit() && runtime->GetJit()->JitAtFirstUse()) {
+    // The force JIT uses the interpreter entry point to execute the JIT.
+    return true;
+  }
+
+  return false;
 }
 
 void ClassLinker::FixupStaticTrampolines(mirror::Class* klass) {
@@ -2677,15 +2695,12 @@ void ClassLinker::FixupStaticTrampolines(mirror::Class* klass) {
       OatFile::OatMethod oat_method = oat_class.GetOatMethod(method_index);
       quick_code = oat_method.GetQuickCode();
     }
-    const bool enter_interpreter = NeedsInterpreter(method, quick_code);
-    if (enter_interpreter) {
+    // Check whether the method is native, in which case it's generic JNI.
+    if (quick_code == nullptr && method->IsNative()) {
+      quick_code = GetQuickGenericJniStub();
+    } else if (ShouldUseInterpreterEntrypoint(method, quick_code)) {
       // Use interpreter entry point.
-      // Check whether the method is native, in which case it's generic JNI.
-      if (quick_code == nullptr && method->IsNative()) {
-        quick_code = GetQuickGenericJniStub();
-      } else {
-        quick_code = GetQuickToInterpreterBridge();
-      }
+      quick_code = GetQuickToInterpreterBridge();
     }
     runtime->GetInstrumentation()->UpdateMethodsCode(method, quick_code);
   }
@@ -2716,7 +2731,8 @@ void ClassLinker::LinkCode(ArtMethod* method, const OatFile::OatClass* oat_class
   }
 
   // Install entry point from interpreter.
-  bool enter_interpreter = NeedsInterpreter(method, method->GetEntryPointFromQuickCompiledCode());
+  const void* quick_code = method->GetEntryPointFromQuickCompiledCode();
+  bool enter_interpreter = ShouldUseInterpreterEntrypoint(method, quick_code);
 
   if (!method->IsInvokable()) {
     EnsureThrowsInvocationError(method);
@@ -2728,20 +2744,18 @@ void ClassLinker::LinkCode(ArtMethod* method, const OatFile::OatClass* oat_class
     // It will be replaced by the proper entry point by ClassLinker::FixupStaticTrampolines
     // after initializing class (see ClassLinker::InitializeClass method).
     method->SetEntryPointFromQuickCompiledCode(GetQuickResolutionStub());
+  } else if (quick_code == nullptr && method->IsNative()) {
+    method->SetEntryPointFromQuickCompiledCode(GetQuickGenericJniStub());
   } else if (enter_interpreter) {
-    if (!method->IsNative()) {
-      // Set entry point from compiled code if there's no code or in interpreter only mode.
-      method->SetEntryPointFromQuickCompiledCode(GetQuickToInterpreterBridge());
-    } else {
-      method->SetEntryPointFromQuickCompiledCode(GetQuickGenericJniStub());
-    }
+    // Set entry point from compiled code if there's no code or in interpreter only mode.
+    method->SetEntryPointFromQuickCompiledCode(GetQuickToInterpreterBridge());
   }
 
   if (method->IsNative()) {
     // Unregistering restores the dlsym lookup stub.
     method->UnregisterNative();
 
-    if (enter_interpreter) {
+    if (enter_interpreter || quick_code == nullptr) {
       // We have a native method here without code. Then it should have either the generic JNI
       // trampoline as entrypoint (non-static), or the resolution trampoline (static).
       // TODO: this doesn't handle all the cases where trampolines may be installed.

diff --git a/runtime/class_linker.h b/runtime/class_linker.h
index 56a868a..a9448f7 100644 (file)
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -592,6 +592,9 @@ class ClassLinker {
       REQUIRES(!Locks::classlinker_classes_lock_)
       SHARED_REQUIRES(Locks::mutator_lock_);
 
+  static bool ShouldUseInterpreterEntrypoint(ArtMethod* method, const void* quick_code)
+      SHARED_REQUIRES(Locks::mutator_lock_);
+
   struct DexCacheData {
     // Weak root to the DexCache. Note: Do not decode this unnecessarily or else class unloading may
     // not work properly.

diff --git a/runtime/interpreter/interpreter.cc b/runtime/interpreter/interpreter.cc
index 0b2471b..4fd3c78 100644 (file)
--- a/runtime/interpreter/interpreter.cc
+++ b/runtime/interpreter/interpreter.cc
@@ -27,6 +27,7 @@
 #include "unstarted_runtime.h"
 #include "mterp/mterp.h"
 #include "jit/jit.h"
+#include "jit/jit_code_cache.h"
 
 namespace art {
 namespace interpreter {
@@ -293,9 +294,10 @@ static inline JValue Execute(Thread* self, const DexFile::CodeItem* code_item,
                                         method, 0);
     }
 
-    if (UNLIKELY(Runtime::Current()->GetJit() != nullptr &&
-                 Runtime::Current()->GetJit()->JitAtFirstUse() &&
-                 method->HasAnyCompiledCode())) {
+    jit::Jit* jit = Runtime::Current()->GetJit();
+    if (UNLIKELY(jit != nullptr &&
+                 jit->JitAtFirstUse() &&
+                 jit->GetCodeCache()->ContainsMethod(method))) {
       JValue result;
 
       // Pop the shadow frame before calling into compiled code.

diff --git a/runtime/interpreter/interpreter_common.cc b/runtime/interpreter/interpreter_common.cc
index 09d8601..cbaa817 100644 (file)
--- a/runtime/interpreter/interpreter_common.cc
+++ b/runtime/interpreter/interpreter_common.cc
@@ -20,6 +20,7 @@
 
 #include "debugger.h"
 #include "entrypoints/runtime_asm_entrypoints.h"
+#include "jit/jit.h"
 #include "mirror/array-inl.h"
 #include "stack.h"
 #include "unstarted_runtime.h"
@@ -501,23 +502,6 @@ static inline bool DoCallCommon(ArtMethod* called_method,
                                 uint32_t (&arg)[kVarArgMax],
                                 uint32_t vregC) ALWAYS_INLINE;
 
-SHARED_REQUIRES(Locks::mutator_lock_)
-static inline bool NeedsInterpreter(Thread* self, ShadowFrame* new_shadow_frame) ALWAYS_INLINE;
-
-static inline bool NeedsInterpreter(Thread* self, ShadowFrame* new_shadow_frame) {
-  ArtMethod* target = new_shadow_frame->GetMethod();
-  if (UNLIKELY(target->IsNative() || target->IsProxyMethod())) {
-    return false;
-  }
-  Runtime* runtime = Runtime::Current();
-  ClassLinker* class_linker = runtime->GetClassLinker();
-  return runtime->GetInstrumentation()->IsForcedInterpretOnly() ||
-        // Doing this check avoids doing compiled/interpreter transitions.
-        class_linker->IsQuickToInterpreterBridge(target->GetEntryPointFromQuickCompiledCode()) ||
-        // Force the use of interpreter when it is required by the debugger.
-        Dbg::IsForcedInterpreterNeededForCalling(self, target);
-}
-
 void ArtInterpreterToCompiledCodeBridge(Thread* self,
                                         const DexFile::CodeItem* code_item,
                                         ShadowFrame* shadow_frame,
@@ -736,7 +720,10 @@ static inline bool DoCallCommon(ArtMethod* called_method,
 
   // Do the call now.
   if (LIKELY(Runtime::Current()->IsStarted())) {
-    if (NeedsInterpreter(self, new_shadow_frame)) {
+    ArtMethod* target = new_shadow_frame->GetMethod();
+    if (ClassLinker::ShouldUseInterpreterEntrypoint(
+        target,
+        target->GetEntryPointFromQuickCompiledCode())) {
       ArtInterpreterToInterpreterBridge(self, code_item, new_shadow_frame, result);
     } else {
       ArtInterpreterToCompiledCodeBridge(self, code_item, new_shadow_frame, result);

diff --git a/runtime/native/dalvik_system_ZygoteHooks.cc b/runtime/native/dalvik_system_ZygoteHooks.cc
index a7881ac..a092b9f 100644 (file)
--- a/runtime/native/dalvik_system_ZygoteHooks.cc
+++ b/runtime/native/dalvik_system_ZygoteHooks.cc
@@ -66,6 +66,7 @@ static void EnableDebugFeatures(uint32_t debug_flags) {
     DEBUG_ENABLE_JNI_LOGGING        = 1 << 4,
     DEBUG_GENERATE_DEBUG_INFO       = 1 << 5,
     DEBUG_ALWAYS_JIT                = 1 << 6,
+    DEBUG_NATIVE_DEBUGGABLE         = 1 << 7,
   };
 
   Runtime* const runtime = Runtime::Current();
@@ -117,6 +118,11 @@ static void EnableDebugFeatures(uint32_t debug_flags) {
     debug_flags &= ~DEBUG_ALWAYS_JIT;
   }
 
+  if ((debug_flags & DEBUG_NATIVE_DEBUGGABLE) != 0) {
+    runtime->AddCompilerOption("--native-debuggable");
+    debug_flags &= ~DEBUG_NATIVE_DEBUGGABLE;
+  }
+
   if (debug_flags != 0) {
     LOG(ERROR) << StringPrintf("Unknown bits set in debug_flags: %#x", debug_flags);
   }