Quick fix for inline caching profiler

Summary: A fix to make inline caching profiler work after the recent changes in internal bytecode. Reviewed By: dulinriley Differential Revision: D17529650 fbshipit-source-id: 82239cc225c61bcc8412e7d810f19444f6e112ca
meplay · Sep 26, 2019 · bd2d76d · bd2d76d
1 parent 29fddeb
commit bd2d76d
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 15 deletions.
diff --git a/include/hermes/VM/Profiler/InlineCacheProfiler.h b/include/hermes/VM/Profiler/InlineCacheProfiler.h
@@ -89,7 +89,7 @@ class InlineCacheProfiler {
 
   /// Get a JS array containing all hidden classes that shouldn't be
   /// garbage collected.
-  JSArray *getHiddenClassArray();
+  JSArray *&getHiddenClassArray();
 
   /// Set a JS array containing all hidden classes that shouldn't be
   /// garbage collected.

diff --git a/lib/VM/Interpreter.cpp b/lib/VM/Interpreter.cpp
@@ -2161,11 +2161,17 @@ CallResult<HermesValue> Interpreter::interpretFunction(
         auto *cacheEntry = curCodeBlock->getReadCacheEntry(cacheIdx);
 
 #ifdef HERMESVM_PROFILER_BB
-        HERMES_SLOW_ASSERT(
-            gcScope.getHandleCountDbg() == KEEP_HANDLES &&
-            "unaccounted handles were created");
-        runtime->recordHiddenClass(
-            curCodeBlock, ip, ID(idVal), clazz, cacheEntry->clazz);
+        {
+          HERMES_SLOW_ASSERT(
+              gcScope.getHandleCountDbg() == KEEP_HANDLES &&
+              "unaccounted handles were created");
+          auto objHandle = runtime->makeHandle(obj);
+          runtime->recordHiddenClass(
+              curCodeBlock, ip, ID(idVal), clazz, cacheEntry->clazz);
+          // obj may be moved by GC due to recordHiddenClass
+          obj = objHandle.get();
+          clazz = obj->getClass(runtime);
+        }
         gcScope.flushToSmallCount(KEEP_HANDLES);
 #endif
         // If we have a cache hit, reuse the cached offset and immediately
@@ -2303,11 +2309,17 @@ CallResult<HermesValue> Interpreter::interpretFunction(
         auto *cacheEntry = curCodeBlock->getWriteCacheEntry(cacheIdx);
 
 #ifdef HERMESVM_PROFILER_BB
-        HERMES_SLOW_ASSERT(
-            gcScope.getHandleCountDbg() == KEEP_HANDLES &&
-            "unaccounted handles were created");
-        runtime->recordHiddenClass(
-            curCodeBlock, ip, ID(idVal), clazz, cacheEntry->clazz);
+        {
+          HERMES_SLOW_ASSERT(
+              gcScope.getHandleCountDbg() == KEEP_HANDLES &&
+              "unaccounted handles were created");
+          auto objHandle = runtime->makeHandle(obj);
+          runtime->recordHiddenClass(
+              curCodeBlock, ip, ID(idVal), clazz, cacheEntry->clazz);
+          // obj may be moved by GC due to recordHiddenClass
+          obj = objHandle.get();
+          clazz = obj->getClass(runtime);
+        }
         gcScope.flushToSmallCount(KEEP_HANDLES);
 #endif
         // If we have a cache hit, reuse the cached offset and immediately

diff --git a/lib/VM/Profiler/InlineCacheProfiler.cpp b/lib/VM/Profiler/InlineCacheProfiler.cpp
@@ -66,7 +66,7 @@ bool InlineCacheProfiler::insertICHit(
   return true;
 }
 
-JSArray *InlineCacheProfiler::getHiddenClassArray() {
+JSArray *&InlineCacheProfiler::getHiddenClassArray() {
   return cachedHiddenClassesRawPtr_;
 }
 

diff --git a/lib/VM/Runtime.cpp b/lib/VM/Runtime.cpp
@@ -513,9 +513,7 @@ void Runtime::markRoots(RootAcceptor &acceptor, bool markLongLived) {
       samplingProfiler_->markRoots(acceptor);
     }
 #ifdef HERMESVM_PROFILER_BB
-    auto *hiddenCalssArray = inlineCacheProfiler_.getHiddenClassArray();
-    acceptor.acceptPtr(hiddenCalssArray, "");
-    inlineCacheProfiler_.setHiddenClassArray(hiddenCalssArray);
+    acceptor.acceptPtr(inlineCacheProfiler_.getHiddenClassArray());
 #endif
     acceptor.endRootSection();
   }