[gjs: 1/4] GjsMaybeOwned: Don't use union for root and heap
- From: Philip Chimento <pchimento src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gjs: 1/4] GjsMaybeOwned: Don't use union for root and heap
- Date: Thu, 20 Jun 2019 18:03:14 +0000 (UTC)
commit f2879d52a51b03b80fe29af48d9a0e488f2c21d8
Author: Marco Trevisan (TreviƱo) <mail 3v1n0 net>
Date: Mon Jun 17 22:35:46 2019 +0200
GjsMaybeOwned: Don't use union for root and heap
Using an union and a boolean used to check which member uses the same memory
of two instances, so just use private members using the pointer to check
the rooted state.
gjs/jsapi-util-root.h | 115 ++++++++++++++++++++++++--------------------------
1 file changed, 54 insertions(+), 61 deletions(-)
---
diff --git a/gjs/jsapi-util-root.h b/gjs/jsapi-util-root.h
index 1358b382..b8d298b9 100644
--- a/gjs/jsapi-util-root.h
+++ b/gjs/jsapi-util-root.h
@@ -1,6 +1,7 @@
/* -*- mode: C++; c-basic-offset: 4; indent-tabs-mode: nil; -*- */
/*
* Copyright (c) 2017 Endless Mobile, Inc.
+ * Copyright (c) 2019 Canonical, Ltd.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to
@@ -27,6 +28,7 @@
#include <stdint.h> // for uintptr_t
#include <cstddef> // for nullptr_t
+#include <memory>
#include <new>
#include <type_traits> // for enable_if_t, is_pointer
@@ -114,25 +116,19 @@ struct GjsHeapOperation<JSFunction*> {
* member of a heap-allocated struct. */
template<typename T>
class GjsMaybeOwned {
-public:
+ public:
typedef void (*DestroyNotify)(JS::Handle<T> thing, void *data);
-private:
- bool m_rooted; /* wrapper is in rooted mode */
+ private:
bool m_has_weakref; /* we have a weak reference to the GjsContext */
JSContext *m_cx;
- /* m_rooted controls which of these members we can access. When switching
+ /* m_root value controls which of these members we can access. When switching
* from one to the other, be careful to call the constructor and destructor
* of JS::Heap, since they use post barriers. */
- union RootUnion {
- JS::Heap<T> heap;
- JS::PersistentRooted<T>* root;
-
- RootUnion() : heap() {}
- ~RootUnion() {}
- } m_thing;
+ JS::Heap<T> m_heap;
+ std::unique_ptr<JS::PersistentRooted<T>> m_root;
DestroyNotify m_notify;
void *m_data;
@@ -151,11 +147,10 @@ private:
void teardown_rooting() {
debug("teardown_rooting()");
- g_assert(m_rooted);
+ g_assert(m_root);
- delete m_thing.root;
- new (&m_thing.heap) JS::Heap<T>();
- m_rooted = false;
+ m_root.reset();
+ new (&m_heap) JS::Heap<T>();
if (!m_has_weakref)
return;
@@ -173,7 +168,7 @@ private:
invalidate(void)
{
debug("invalidate()");
- g_assert(m_rooted);
+ g_assert(m_root);
/* The weak ref is already gone because the context is dead, so no need
* to remove it. */
@@ -187,24 +182,23 @@ private:
reset();
}
-public:
- GjsMaybeOwned(void) :
- m_rooted(false),
- m_has_weakref(false),
- m_cx(nullptr),
- m_notify(nullptr),
- m_data(nullptr)
- {
+ public:
+ GjsMaybeOwned()
+ : m_has_weakref(false),
+ m_cx(nullptr),
+ m_root(nullptr),
+ m_notify(nullptr),
+ m_data(nullptr) {
debug("created");
}
~GjsMaybeOwned() {
debug("destroyed");
- if (m_rooted)
+ if (m_root)
teardown_rooting();
/* Call in either case; teardown_rooting() constructs a new Heap */
- m_thing.heap.~Heap();
+ m_heap.~Heap();
}
/* To access the GC thing, call get(). In many cases you can just use the
@@ -212,7 +206,7 @@ public:
* cast operator. But if you want to call methods on the GC thing, for
* example if it's a JS::Value, you have to use get(). */
GJS_USE const T get() const {
- return m_rooted ? m_thing.root->get() : m_thing.heap.get();
+ return m_root ? m_root->get() : m_heap.get();
}
operator const T() const { return get(); }
@@ -220,15 +214,15 @@ public:
template <typename U = T>
GJS_USE const T
debug_addr(std::enable_if_t<std::is_pointer<U>::value>* = nullptr) const {
- return m_rooted ? m_thing.root->get() : m_thing.heap.unbarrieredGet();
+ return m_root ? m_root->get() : m_heap.unbarrieredGet();
}
bool
operator==(const T& other) const
{
- if (m_rooted)
- return m_thing.root->get() == other;
- return m_thing.heap == other;
+ if (m_root)
+ return m_root->get() == other;
+ return m_heap == other;
}
inline bool operator!=(const T& other) const { return !(*this == other); }
@@ -237,9 +231,9 @@ public:
bool
operator==(std::nullptr_t) const
{
- if (m_rooted)
- return m_thing.root->get() == nullptr;
- return m_thing.heap.unbarrieredGet() == nullptr;
+ if (m_root)
+ return m_root->get() == nullptr;
+ return m_heap.unbarrieredGet() == nullptr;
}
inline bool operator!=(std::nullptr_t) const { return !(*this == nullptr); }
@@ -250,8 +244,8 @@ public:
* wrapper with stack rooting. However, you must not do this if the
* JSContext can be destroyed while the Handle is live. */
GJS_USE JS::Handle<T> handle() {
- g_assert(m_rooted);
- return *m_thing.root;
+ g_assert(m_root);
+ return *m_root;
}
/* Roots the GC thing. You must not use this if you're already using the
@@ -263,14 +257,13 @@ public:
void *data = nullptr)
{
debug("root()");
- g_assert(!m_rooted);
- g_assert(m_thing.heap.get() == JS::GCPolicy<T>::initial());
- m_rooted = true;
+ g_assert(!m_root);
+ g_assert(m_heap.get() == JS::GCPolicy<T>::initial());
m_cx = cx;
m_notify = notify;
m_data = data;
- m_thing.heap.~Heap();
- m_thing.root = new JS::PersistentRooted<T>(m_cx, thing);
+ m_heap.~Heap();
+ m_root = std::make_unique<JS::PersistentRooted<T>>(m_cx, thing);
if (notify) {
GjsContextPrivate* gjs = GjsContextPrivate::from_cx(m_cx);
@@ -286,8 +279,8 @@ public:
void
operator=(const T& thing)
{
- g_assert(!m_rooted);
- m_thing.heap = thing;
+ g_assert(!m_root);
+ m_heap = thing;
}
/* Marks an object as reachable for one GC with ExposeObjectToActiveJS().
@@ -295,14 +288,14 @@ public:
* in the rooted case. */
void prevent_collection() {
debug("prevent_collection()");
- g_assert(!m_rooted);
- GjsHeapOperation<T>::expose_to_js(m_thing.heap);
+ g_assert(!m_root);
+ GjsHeapOperation<T>::expose_to_js(m_heap);
}
void reset() {
debug("reset()");
- if (!m_rooted) {
- m_thing.heap = JS::GCPolicy<T>::initial();
+ if (!m_root) {
+ m_heap = JS::GCPolicy<T>::initial();
return;
}
@@ -318,30 +311,30 @@ public:
void *data = nullptr)
{
debug("switch to rooted");
- g_assert(!m_rooted);
+ g_assert(!m_root);
/* Prevent the thing from being garbage collected while it is in neither
- * m_thing.heap nor m_thing.root */
+ * m_heap nor m_root */
JSAutoRequest ar(cx);
- JS::Rooted<T> thing(cx, m_thing.heap);
+ JS::Rooted<T> thing(cx, m_heap);
reset();
root(cx, thing, notify, data);
- g_assert(m_rooted);
+ g_assert(m_root);
}
void switch_to_unrooted() {
debug("switch to unrooted");
- g_assert(m_rooted);
+ g_assert(m_root);
/* Prevent the thing from being garbage collected while it is in neither
- * m_thing.heap nor m_thing.root */
+ * m_heap nor m_root */
JSAutoRequest ar(m_cx);
- JS::Rooted<T> thing(m_cx, *m_thing.root);
+ JS::Rooted<T> thing(m_cx, *m_root);
reset();
- m_thing.heap = thing;
- g_assert(!m_rooted);
+ m_heap = thing;
+ g_assert(!m_root);
}
/* Tracing makes no sense in the rooted case, because JS::PersistentRooted
@@ -351,8 +344,8 @@ public:
const char *name)
{
debug("trace()");
- g_assert(!m_rooted);
- JS::TraceEdge<T>(tracer, &m_thing.heap, name);
+ g_assert(!m_root);
+ JS::TraceEdge<T>(tracer, &m_heap, name);
}
/* If not tracing, then you must call this method during GC in order to
@@ -360,11 +353,11 @@ public:
* finalized. If the object was finalized, returns true. */
GJS_USE bool update_after_gc() {
debug("update_after_gc()");
- g_assert(!m_rooted);
- return GjsHeapOperation<T>::update_after_gc(&m_thing.heap);
+ g_assert(!m_root);
+ return GjsHeapOperation<T>::update_after_gc(&m_heap);
}
- GJS_USE bool rooted() const { return m_rooted; }
+ GJS_USE bool rooted() const { return m_root != nullptr; }
};
#endif // GJS_JSAPI_UTIL_ROOT_H_
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]