Skip to content

Commit bb7a344

Browse files
GeequlimGeequlim
committed
Improve RefCounted binding
[ci skip]
1 parent 56016ee commit bb7a344

File tree

2 files changed

+31
-55
lines changed

2 files changed

+31
-55
lines changed

‎ecmascript_gc_handler.h

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -17,30 +17,24 @@ struct ECMAScriptGCHandler {
1717
FLAG_ATOMIC_VALUE = 1 << 1,
1818
FLAG_BUILTIN_CLASS = 1 << 2,
1919
FLAG_OBJECT = 1 << 3,
20-
FLAG_REFERENCE = 1 << 4,
21-
FLAG_SCRIPT_FINALIZED = 1 << 5,
22-
FLAG_CONTEXT_TRANSFERABLE = 1 << 6,
20+
FLAG_REF_COUNTED = 1 << 4,
21+
FLAG_FINALIZED = 1 << 5,
22+
FLAG_TRANSFERABLE = 1 << 6,
2323
};
2424
Variant::Type type;
2525
uint8_t flags;
2626
void *context;
2727
void *ecma_object;
2828
union {
2929
Object *godot_object;
30-
Ref<RefCounted> *godot_reference;
3130
void *godot_builtin_object_ptr;
3231
void *native_ptr;
3332
};
3433

3534
_FORCE_INLINE_ Variant get_value() const {
3635
switch (type) {
37-
case Variant::OBJECT: {
38-
if (flags & FLAG_REFERENCE) {
39-
return *godot_reference;
40-
} else if (flags & FLAG_OBJECT) {
41-
return godot_object;
42-
}
43-
} break;
36+
case Variant::OBJECT:
37+
return godot_object;
4438
case Variant::VECTOR2:
4539
return *(static_cast<Vector2 *>(godot_builtin_object_ptr));
4640
case Variant::RECT2:
@@ -88,12 +82,10 @@ struct ECMAScriptGCHandler {
8882
}
8983

9084
_FORCE_INLINE_ Object *get_godot_object() {
91-
if (flags & FLAG_REFERENCE && godot_reference) {
92-
return godot_reference->ptr();
93-
} else if (flags & FLAG_OBJECT) {
85+
if (flags & FLAG_OBJECT) {
9486
return godot_object;
9587
}
96-
return NULL;
88+
return nullptr;
9789
}
9890

9991
_FORCE_INLINE_ Vector2 *getVector2() const { return static_cast<Vector2 *>(godot_builtin_object_ptr); }
@@ -118,23 +110,23 @@ struct ECMAScriptGCHandler {
118110
_FORCE_INLINE_ PackedVector3Array *getPackedVector3Array() const { return static_cast<PackedVector3Array *>(godot_builtin_object_ptr); }
119111

120112
_FORCE_INLINE_ bool is_object() const {
121-
return flags & FLAG_OBJECT && !(flags & FLAG_REFERENCE);
113+
return flags & FLAG_OBJECT;
122114
}
123115

124-
_FORCE_INLINE_ bool is_reference() const {
125-
return flags & FLAG_REFERENCE;
116+
_FORCE_INLINE_ bool is_ref_counted() const {
117+
return flags & FLAG_REF_COUNTED;
126118
}
127119

128120
_FORCE_INLINE_ bool is_transferable() const {
129-
return flags & FLAG_CONTEXT_TRANSFERABLE;
121+
return flags & FLAG_TRANSFERABLE;
130122
}
131123

132124
_FORCE_INLINE_ bool is_atomic_type() const {
133125
return flags & FLAG_ATOMIC_VALUE;
134126
}
135127

136128
_FORCE_INLINE_ bool is_finalized() const {
137-
return flags & FLAG_SCRIPT_FINALIZED;
129+
return flags & FLAG_FINALIZED;
138130
}
139131
_FORCE_INLINE_ bool is_valid_ecma_object() const {
140132
return context != NULL && ecma_object != NULL && !is_finalized();

‎quickjs/quickjs_binder.cpp

Lines changed: 19 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,6 @@ JSValue QuickJSBinder::variant_to_var(JSContext *ctx, const Variant p_var) {
247247
ERR_FAIL_NULL_V(data, JS_UNDEFINED);
248248
ERR_FAIL_NULL_V(data->ecma_object, JS_UNDEFINED);
249249
ERR_FAIL_COND_V(data->context != ctx, (JS_UNDEFINED));
250-
QuickJSBinder *binder = get_context_binder(ctx);
251250
JSValue js_obj = JS_MKPTR(JS_TAG_OBJECT, data->ecma_object);
252251
JS_DupValue(ctx, js_obj);
253252

@@ -555,9 +554,7 @@ void QuickJSBinder::add_debug_binding_info(JSContext *ctx, JSValueConst p_obj, c
555554
if (p_bind->type != Variant::OBJECT) {
556555
classname = to_js_string(ctx, Variant::get_type_name(p_bind->type));
557556
} else {
558-
if (p_bind->is_reference()) {
559-
classname = to_js_string(ctx, (*p_bind->godot_reference)->get_class_name());
560-
} else if (p_bind->is_object()) {
557+
if (p_bind->is_object()) {
561558
classname = to_js_string(ctx, p_bind->godot_object->get_class_name());
562559
}
563560
}
@@ -1558,7 +1555,7 @@ ECMAScriptGCHandler *QuickJSBinder::alloc_object_binding_data(Object *p_object)
15581555

15591556
void QuickJSBinder::free_object_binding_data(ECMAScriptGCHandler *p_gc_handle) {
15601557
ECMAScriptGCHandler *bind = (ECMAScriptGCHandler *)p_gc_handle;
1561-
if (bind->is_object()) {
1558+
if (!bind->is_ref_counted()) {
15621559
JSValue js_obj = JS_MKPTR(JS_TAG_OBJECT, bind->ecma_object);
15631560
JS_SetOpaque(js_obj, NULL);
15641561
JS_FreeValue((JSContext *)bind->context, js_obj);
@@ -1585,21 +1582,9 @@ Error QuickJSBinder::bind_gc_object(JSContext *ctx, ECMAScriptGCHandler *data, O
15851582
data->type = Variant::OBJECT;
15861583
data->flags = ECMAScriptGCHandler::FLAG_OBJECT;
15871584
if (p_object->is_ref_counted()) {
1588-
RefCounted *ref = Object::cast_to<RefCounted>(p_object);
1589-
data->flags |= ECMAScriptGCHandler::FLAG_REFERENCE;
1590-
#if 0
1591-
union {
1592-
Ref<RefCounted> *ref;
1593-
struct {
1594-
RefCounted *ref;
1595-
} * r;
1596-
} u;
1597-
u.ref = memnew(Ref<RefCounted>);
1598-
u.r->ref = ref;
1599-
data->godot_reference = u.ref;
1600-
#else
1601-
data->godot_reference = memnew(Ref<RefCounted>(ref));
1602-
#endif
1585+
if (static_cast<RefCounted *>(p_object)->init_ref()) {
1586+
data->flags |= ECMAScriptGCHandler::FLAG_REF_COUNTED;
1587+
}
16031588
}
16041589
JS_SetOpaque(obj, data);
16051590
#ifdef DUMP_LEAKS
@@ -1612,14 +1597,14 @@ Error QuickJSBinder::bind_gc_object(JSContext *ctx, ECMAScriptGCHandler *data, O
16121597
}
16131598

16141599
void QuickJSBinder::godot_refcount_incremented(ECMAScriptGCHandler *bind) {
1615-
if (bind->is_valid_ecma_object() && bind->is_reference()) {
1600+
if (bind->is_valid_ecma_object()) {
16161601
JSValue js_obj = JS_MKPTR(JS_TAG_OBJECT, bind->ecma_object);
16171602
JS_DupValue((JSContext *)bind->context, js_obj);
16181603
}
16191604
}
16201605

16211606
bool QuickJSBinder::godot_refcount_decremented(ECMAScriptGCHandler *bind) {
1622-
if (bind->is_valid_ecma_object() && bind->is_reference()) {
1607+
if (bind->is_valid_ecma_object()) {
16231608
JSValue js_obj = JS_MKPTR(JS_TAG_OBJECT, bind->ecma_object);
16241609
JS_FreeValue((JSContext *)bind->context, js_obj);
16251610
return bind->is_finalized();
@@ -1669,9 +1654,7 @@ JSValue QuickJSBinder::object_constructor(JSContext *ctx, JSValueConst new_targe
16691654
JS_FreeValue(ctx, es_class_name);
16701655
gd_obj->set_script_instance(si);
16711656
#endif
1672-
if (bind->is_reference()) {
1673-
bind->flags |= ECMAScriptGCHandler::FLAG_REFERENCE;
1674-
} else if (bind->is_object()) {
1657+
if (!bind->is_ref_counted()) { // Object need to be freed manually
16751658
JS_DupValue(ctx, js_obj);
16761659
}
16771660
}
@@ -1696,17 +1679,19 @@ void QuickJSBinder::initialize_properties(JSContext *ctx, const ECMAClassInfo *p
16961679

16971680
void QuickJSBinder::object_finalizer(ECMAScriptGCHandler *p_bind) {
16981681
p_bind->flags ^= ECMAScriptGCHandler::FLAG_OBJECT;
1699-
if (p_bind->is_reference()) {
1700-
p_bind->flags ^= ECMAScriptGCHandler::FLAG_REFERENCE;
1701-
memdelete(p_bind->godot_reference);
1682+
if (p_bind->godot_object->is_ref_counted()) {
1683+
RefCounted *ref = static_cast<RefCounted *>(p_bind->godot_object);
1684+
if (ref->unreference()) {
1685+
memdelete(ref);
1686+
}
17021687
}
17031688
}
17041689

17051690
void QuickJSBinder::origin_finalizer(JSRuntime *rt, JSValue val) {
17061691
QuickJSBinder *binder = get_runtime_binder(rt);
17071692
ECMAScriptGCHandler *bind = static_cast<ECMAScriptGCHandler *>(JS_GetOpaque(val, binder->godot_origin_class.class_id));
17081693
if (bind) {
1709-
bind->flags |= ECMAScriptGCHandler::FLAG_SCRIPT_FINALIZED;
1694+
bind->flags |= ECMAScriptGCHandler::FLAG_FINALIZED;
17101695
if (bind->type == Variant::OBJECT) {
17111696
object_finalizer(bind);
17121697
} else {
@@ -1720,7 +1705,7 @@ JSValue QuickJSBinder::object_free(JSContext *ctx, JSValue this_val, int argc, J
17201705
ECMAScriptGCHandler *bind = BINDING_DATA_FROM_JS(ctx, this_val);
17211706
ERR_FAIL_NULL_V(bind, JS_ThrowReferenceError(ctx, "The object already be freed"));
17221707
ERR_FAIL_NULL_V(bind->godot_object, JS_ThrowReferenceError(ctx, "The object already be freed"));
1723-
ERR_FAIL_COND_V((bind->is_reference()), JS_ThrowReferenceError(ctx, "Call free to RefCounted object is not allowed"));
1708+
ERR_FAIL_COND_V(bind->godot_object->is_ref_counted(), JS_ThrowReferenceError(ctx, "Call free to RefCounted object is not allowed"));
17241709

17251710
memdelete(bind->godot_object);
17261711
JS_SetOpaque(this_val, NULL);
@@ -2266,16 +2251,15 @@ JSValue QuickJSBinder::godot_abandon_value(JSContext *ctx, JSValue this_val, int
22662251
ECMAScriptGCHandler *data = BINDING_DATA_FROM_JS(ctx, value);
22672252
if (data) {
22682253
JS_SetOpaque(value, NULL);
2269-
if (data->type == Variant::OBJECT) {
2254+
if (data->is_ref_counted()) {
2255+
static_cast<RefCounted *>(data->godot_object)->unreference();
2256+
} else if (data->is_object()) {
22702257
JS_FreeValue(ctx, value);
2271-
if (data->is_reference()) {
2272-
memdelete(data->godot_reference);
2273-
}
22742258
}
22752259
data->godot_object = NULL;
22762260
data->ecma_object = NULL;
22772261
data->context = NULL;
2278-
data->flags |= ECMAScriptGCHandler::FLAG_CONTEXT_TRANSFERABLE;
2262+
data->flags |= ECMAScriptGCHandler::FLAG_TRANSFERABLE;
22792263
} else {
22802264
valid = false;
22812265
}

0 commit comments

Comments
 (0)