Introduce EmbedderHeapTracer

include/v8.h

Index: include/v8.h
diff --git a/include/v8.h b/include/v8.h
index 6f2bba09bf14d88060da0225f417f3d7b40df098..039c6a77cf2b684b56a076cd597be91738cbf861 100644
--- a/include/v8.h
+++ b/include/v8.h
@@ -5158,6 +5158,8 @@ enum GCCallbackFlags {
 };
 
 typedef void (*GCCallback)(GCType type, GCCallbackFlags flags);
+typedef void (*GCAliveHandleCallback)(Persistent<Object>* handle,

[jochen (gone - plz use gerrit), 2016/03/21 13:31:15]

why not add a new method on PersistentBase?

[Marcel Hlopko, 2016/03/21 14:29:58]

Done.

+                                      Isolate* isolate);
 
 typedef void (*InterruptCallback)(Isolate* isolate, void* data);
 
@@ -5350,6 +5352,25 @@ class V8_EXPORT PersistentHandleVisitor {  // NOLINT
                                      uint16_t class_id) {}
 };
 
+/**
+ * Interface for tracing through the embedder heap. During the v8 garbage
+ * collection, the embedder is notified that some object he has a reference to
+ * is still alive, and is expected to trace through its heap and call `callback`
+ * function for each dependent js object.
+ *
+ * After the v8 garbage collection is finished, `ClearTracingMarks` is called so
+ * the embedder can clear its temporary data.
+ */
+class V8_EXPORT EmbedderHeapTracer {  // NOLINT

[jochen (gone - plz use gerrit), 2016/03/21 13:31:15]

what is // NOLINT needed for?

since the class doesn't have anything outside of this header, I don't think you
need to V8_EXPORT it?

[Marcel Hlopko, 2016/03/21 14:29:58]

Done.

+ public:
+  virtual ~EmbedderHeapTracer() {}

[jochen (gone - plz use gerrit), 2016/03/21 13:31:15]

should this be protected?

+  virtual void TraceWrappablesFrom(Isolate* isolate, Persistent<Object>* value,

[jochen (gone - plz use gerrit), 2016/03/21 13:31:15]

Do you expect this method to return immediately? Do you expect it gets invoked
often?

I wonder whether instead of passing a single persistent, we should pass a vector
of internal fields (because that's essentially what the embedder needs to
identify the object the handle points out).

I'd also call it something like "PutOnMarkingDeque" or similar.

in any case, please document what the method should do.

also, should it be pure virtual?

[Marcel Hlopko, 2016/03/21 14:29:58]

I added the comment, made the method pure virtual. 

Regarding internal fields, sure can do, I just think passing persistent "feels"
nicer and more reusable. I understand though that yagni to be more reusable.

Regarding PutOnMarkingDeque, I changed the name of the callback, I think this is
much better name that what I came with. If you don't like TraceWrappableFrom,
does TraceDependantsOf sound better?

+                                   GCAliveHandleCallback callback) {}
+  /**
+   * V8 allocation is *not* allowed in the `ClearTracingMarks`.
+   */
+  virtual void ClearTracingMarks(Isolate* isolate) {}
+};
 
 /**
  * Isolate represents an isolated instance of the V8 engine.  V8 isolates have
@@ -5812,6 +5833,17 @@ class V8_EXPORT Isolate {
   void RemoveGCPrologueCallback(GCCallback callback);
 
   /**
+   * Sets the embedder heap tracer for the isolate.
+   */
+  void SetEmbedderHeapTracer(EmbedderHeapTracer* tracer);
+
+  /**
+   * This function removes the tracer installed by `SetEmbedderHeapTracer`

[jochen (gone - plz use gerrit), 2016/03/21 13:31:15]

please no backticks

you could require the EmbedderHeapTracer* to be passed in again, so you can
check that the embedder is removing the right one.

however, I wonder whether we should allow removing it at all...

[Marcel Hlopko, 2016/03/21 14:29:58]

I will gladly remove the method, I don't think it will ever be used in practice.
I added it only to go with the flow with other GC callbacks (where Remove
methods are actually useful).

I was thinking about introducing NoopEmpedderHeapTracer and setting it as a
default in an isolate, so we don't have to nullcheck each time we access tracer.
But as the code is not too bad with the nullchecks, I haven't done so yet. WDYT?

+   * function.
+   */
+  void UnsetEmbedderHeapTracer();
+
+  /**
    * Enables the host application to receive a notification after a
    * garbage collection. Allocations are allowed in the callback function,
    * but the callback is not re-entrant: if the allocation inside it will