From 0c7defcf64df0dfdf9e237e2d9287b88de00d629 Mon Sep 17 00:00:00 2001
From: Riley Dulin <dulinr@fb.com>
Date: Fri, 13 Sep 2019 18:45:37 -0700
Subject: [PATCH] Remove jsi::Value return from
 jsi::Instrumentation::getHeapInfo

Summary:
`getHeapInfo` was creating and return a `jsi::Object`. This object was not being traced
when API tracing was turned on.
This resulted in a bug in synth traces, an object was being acted upon without its creation source
being logged.
In order to prevent this, change the API of `getHeapInfo` to no longer return any objects, or modify other heap
contents.

This is preferable to having `TracingRuntime` attempt to mimic the operations of two different implementations.

## Changelog:

[Internal] [Changed] - Changed `jsi::Instrumentation::getHeapInfo` to use a `std::unordered_map`

Reviewed By: mhorowitz

Differential Revision: D17273235

fbshipit-source-id: f69860dcc524c2cf501746a41dbac20b4db8c456
---
 ReactCommon/jsi/jsi/decorator.h       |  3 ++-
 ReactCommon/jsi/jsi/instrumentation.h | 10 +++++++---
 ReactCommon/jsi/jsi/jsi.cpp           |  4 ++--
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/ReactCommon/jsi/jsi/decorator.h b/ReactCommon/jsi/jsi/decorator.h
index b31615212d..819ac5cdf6 100644
--- a/ReactCommon/jsi/jsi/decorator.h
+++ b/ReactCommon/jsi/jsi/decorator.h
@@ -325,7 +325,8 @@ class RuntimeDecorator : public Base, private jsi::Instrumentation {
     return plain().instrumentation().getRecordedGCStats();
   }
 
-  Value getHeapInfo(bool includeExpensive) override {
+  std::unordered_map<std::string, int64_t> getHeapInfo(
+      bool includeExpensive) override {
     return plain().instrumentation().getHeapInfo(includeExpensive);
   }
 
diff --git a/ReactCommon/jsi/jsi/instrumentation.h b/ReactCommon/jsi/jsi/instrumentation.h
index 0840225440..9e9c980315 100644
--- a/ReactCommon/jsi/jsi/instrumentation.h
+++ b/ReactCommon/jsi/jsi/instrumentation.h
@@ -8,6 +8,7 @@
 
 #include <iosfwd>
 #include <string>
+#include <unordered_map>
 
 #include <jsi/jsi.h>
 
@@ -17,6 +18,8 @@ namespace jsi {
 /// Methods for starting and collecting instrumentation, an \c Instrumentation
 /// instance is associated with a particular \c Runtime instance, which it
 /// controls the instrumentation of.
+/// None of these functions should return newly created jsi values, nor should
+/// it modify the values of any jsi values in the heap (although GCs are fine).
 class Instrumentation {
  public:
   virtual ~Instrumentation() = default;
@@ -40,9 +43,10 @@ class Instrumentation {
   /// function can be called at any time, and should produce information that is
   /// correct at the instant it is called (i.e, not stale).
   ///
-  /// \return a jsi Value containing whichever statistics the runtime supports
-  ///   for its heap.
-  virtual Value getHeapInfo(bool includeExpensive) = 0;
+  /// \return a map from a string key to a number associated with that
+  /// statistic.
+  virtual std::unordered_map<std::string, int64_t> getHeapInfo(
+      bool includeExpensive) = 0;
 
   /// perform a full garbage collection
   virtual void collectGarbage() = 0;
diff --git a/ReactCommon/jsi/jsi/jsi.cpp b/ReactCommon/jsi/jsi/jsi.cpp
index 408b321c54..0b8cb80b38 100644
--- a/ReactCommon/jsi/jsi/jsi.cpp
+++ b/ReactCommon/jsi/jsi/jsi.cpp
@@ -71,8 +71,8 @@ Instrumentation& Runtime::instrumentation() {
       return "";
     }
 
-    Value getHeapInfo(bool) override {
-      return Value::undefined();
+    std::unordered_map<std::string, int64_t> getHeapInfo(bool) override {
+      return std::unordered_map<std::string, int64_t>{};
     }
 
     void collectGarbage() override {}