[gjs: 1/26] gtype: Port weak pointer hash table to WeakCache

[Cosimo Cecchi <cosimoc src gnome org>]

commit c7e383435d487a176248163fe96b91d40659fb36
Author: Philip Chimento <philip chimento gmail com>
Date:   Wed Jan 2 18:45:18 2019 -0700

    gtype: Port weak pointer hash table to WeakCache
    
    As in a previous change in fundamental.cpp, JS::WeakCache allows us to
    get rid of the weak pointer callback, as that is all implemented
    internally. The hash table is automatically swept when the GType wrapper
    objects are garbage collected.
    
    We move the table to GjsContextPrivate instead of using a static
    variable. Although there is probably little difference in practice, that
    is a more accurate place to store it, in case there is more than one
    GjsContext in use.

 gi/gtype.cpp          | 55 +++++++++------------------------------------------
 gjs/context-private.h | 20 ++++++++++++++++++-
 gjs/context.cpp       |  7 +++++++
 3 files changed, 35 insertions(+), 47 deletions(-)
---
diff --git a/gi/gtype.cpp b/gi/gtype.cpp
index d89e4377..9d3263c0 100644
--- a/gi/gtype.cpp
+++ b/gi/gtype.cpp
@@ -34,10 +34,6 @@
 #include "gjs/jsapi-wrapper.h"
 #include "util/log.h"
 
-static bool weak_pointer_callback = false;
-static std::unordered_map<GType, std::unique_ptr<JS::Heap<JSObject*>>>
-    weak_pointer_list;
-
 GJS_USE static JSObject* gjs_gtype_get_proto(JSContext* cx) G_GNUC_UNUSED;
 GJS_JSAPI_RETURN_CONVENTION
 static bool gjs_gtype_define_proto(JSContext *, JS::HandleObject,
@@ -49,34 +45,6 @@ GJS_DEFINE_PROTO_ABSTRACT("GIRepositoryGType", gtype,
 /* priv_from_js adds a "*", so this returns "void *" */
 GJS_DEFINE_PRIV_FROM_JS(void, gjs_gtype_class);
 
-static void
-update_gtype_weak_pointers(JSContext     *cx,
-                           JSCompartment *compartment,
-                           void          *data)
-{
-    for (auto iter = weak_pointer_list.begin(); iter != weak_pointer_list.end(); ) {
-        JS::Heap<JSObject *> *heap_wrapper = iter->second.get();
-        JS_UpdateWeakPointerAfterGC(heap_wrapper);
-
-        /* No read barriers are needed if the only thing we are doing with the
-         * pointer is comparing it to nullptr. */
-        if (heap_wrapper->unbarrieredGet() == nullptr)
-            iter = weak_pointer_list.erase(iter);
-        else
-            iter++;
-    }
-}
-
-static void
-ensure_weak_pointer_callback(JSContext *cx)
-{
-    if (!weak_pointer_callback) {
-        JS_AddWeakPointerCompartmentCallback(cx, update_gtype_weak_pointers,
-                                             nullptr);
-        weak_pointer_callback = true;
-    }
-}
-
 static void
 gjs_gtype_finalize(JSFreeOp *fop,
                    JSObject *obj)
@@ -86,8 +54,6 @@ gjs_gtype_finalize(JSFreeOp *fop,
     /* proto doesn't have a private set */
     if (G_UNLIKELY(gtype == 0))
         return;
-
-    weak_pointer_list.erase(gtype);
 }
 
 GJS_JSAPI_RETURN_CONVENTION
@@ -153,26 +119,23 @@ gjs_gtype_create_gtype_wrapper (JSContext *context,
 
     JSAutoRequest ar(context);
 
-    auto heap_wrapper_it = weak_pointer_list.find(gtype);
-    if (heap_wrapper_it != std::end(weak_pointer_list))
-        return *heap_wrapper_it->second;
+    GjsContextPrivate* gjs = GjsContextPrivate::from_cx(context);
+    auto p = gjs->gtype_table().lookupForAdd(gtype);
+    if (p)
+        return p->value();
 
     JS::RootedObject proto(context);
     if (!gjs_gtype_define_proto(context, nullptr, &proto))
         return nullptr;
 
-    auto heap_wrapper = std::make_unique<JS::Heap<JSObject *>>();
-    if (!(*heap_wrapper =
-          JS_NewObjectWithGivenProto(context, &gjs_gtype_class, proto)))
+    JS::RootedObject gtype_wrapper(
+        context, JS_NewObjectWithGivenProto(context, &gjs_gtype_class, proto));
+    if (!gtype_wrapper)
         return nullptr;
 
-    JS_SetPrivate(*heap_wrapper, GSIZE_TO_POINTER(gtype));
-    ensure_weak_pointer_callback(context);
+    JS_SetPrivate(gtype_wrapper, GSIZE_TO_POINTER(gtype));
 
-    /* Saving a reference to the wrapper pointer, as heap_wrapper will be
-     * nullified by std::move */
-    JSObject *gtype_wrapper = *heap_wrapper;
-    weak_pointer_list.insert({gtype, std::move(heap_wrapper)});
+    gjs->gtype_table().add(p, gtype, gtype_wrapper);
 
     return gtype_wrapper;
 }
diff --git a/gjs/context-private.h b/gjs/context-private.h
index 566be990..48131b92 100644
--- a/gjs/context-private.h
+++ b/gjs/context-private.h
@@ -34,17 +34,31 @@
 #include "jsapi-wrapper.h"
 
 #include "js/GCHashTable.h"
+#include "js/GCPolicyAPI.h"
 #include "js/SweepingAPI.h"
 
 using JobQueue = JS::GCVector<JSObject*, 0, js::SystemAllocPolicy>;
 using FundamentalTable =
     JS::GCHashMap<void*, JS::Heap<JSObject*>, js::DefaultHasher<void*>,
                   js::SystemAllocPolicy>;
+using GTypeTable =
+    JS::GCHashMap<GType, JS::Heap<JSObject*>, js::DefaultHasher<GType>,
+                  js::SystemAllocPolicy>;
+
+struct Dummy {};
+using GTypeNotUint64 =
+    std::conditional_t<!std::is_same<GType, uint64_t>::value, GType, Dummy>;
 
-// FundamentalTable's key type is void*, the GC sweep method should ignore it
+// The GC sweep method should ignore FundamentalTable and GTypeTable's key types
 namespace JS {
 template <>
 struct GCPolicy<void*> : public IgnoreGCPolicy<void*> {};
+// We need GCPolicy<GType> for GTypeTable. SpiderMonkey already defines
+// GCPolicy<uint64_t> which is equal to GType on some systems; for others we
+// need to define it. (macOS's uint64_t is unsigned long long, which is a
+// different type from unsigned long, even if they are the same width)
+template <>
+struct GCPolicy<GTypeNotUint64> : public IgnoreGCPolicy<GTypeNotUint64> {};
 }  // namespace JS
 
 class GjsContextPrivate {
@@ -83,6 +97,7 @@ class GjsContextPrivate {
 
     // Weak pointer mapping from fundamental native pointer to JSObject
     JS::WeakCache<FundamentalTable>* m_fundamental_table;
+    JS::WeakCache<GTypeTable>* m_gtype_table;
 
     uint8_t m_exit_code;
 
@@ -140,6 +155,9 @@ class GjsContextPrivate {
     GJS_USE JS::WeakCache<FundamentalTable>& fundamental_table(void) {
         return *m_fundamental_table;
     }
+    GJS_USE JS::WeakCache<GTypeTable>& gtype_table(void) {
+        return *m_gtype_table;
+    }
     GJS_USE
     static const GjsAtoms& atoms(JSContext* cx) { return from_cx(cx)->m_atoms; }
 
diff --git a/gjs/context.cpp b/gjs/context.cpp
index e99d1af3..b2783375 100644
--- a/gjs/context.cpp
+++ b/gjs/context.cpp
@@ -356,6 +356,8 @@ void GjsContextPrivate::dispose(void) {
 
         gjs_debug(GJS_DEBUG_CONTEXT, "Releasing cached JS wrappers");
         m_fundamental_table->clear();
+        m_gtype_table->clear();
+
         /* Do a full GC here before tearing down, since once we do
          * that we may not have the JS_GetPrivate() to access the
          * context
@@ -387,6 +389,7 @@ void GjsContextPrivate::dispose(void) {
         gjs_debug(GJS_DEBUG_CONTEXT, "Freeing allocated resources");
         delete m_job_queue;
         delete m_fundamental_table;
+        delete m_gtype_table;
 
         /* Tear down JS */
         JS_DestroyContext(m_cx);
@@ -471,6 +474,10 @@ GjsContextPrivate::GjsContextPrivate(JSContext* cx, GjsContext* public_context)
     if (!m_fundamental_table->init())
         g_error("Failed to initialize fundamental objects table");
 
+    m_gtype_table = new JS::WeakCache<GTypeTable>(rt);
+    if (!m_gtype_table->init())
+        g_error("Failed to initialize GType objects table");
+
     JS_BeginRequest(m_cx);
 
     JS::RootedObject global(m_cx, gjs_create_global_object(m_cx));