From 80781c4dddf0ba1fdac669f50d9607947754469d Mon Sep 17 00:00:00 2001 From: Sami Vaarala Date: Wed, 12 Oct 2016 20:41:26 +0300 Subject: [PATCH 1/2] Rework internal property definition sites a bit --- src-input/duk_api_internal.h | 2 +- src-input/duk_api_object.c | 11 +++--- src-input/duk_bi_function.c | 4 +-- src-input/duk_hobject_props.c | 45 ------------------------- src-input/duk_hthread_builtins.c | 57 ++++++++++++++++---------------- src-input/duk_js_call.c | 4 +-- src-input/duk_js_var.c | 4 +-- 7 files changed, 41 insertions(+), 86 deletions(-) diff --git a/src-input/duk_api_internal.h b/src-input/duk_api_internal.h index 357ee721..f32dc657 100644 --- a/src-input/duk_api_internal.h +++ b/src-input/duk_api_internal.h @@ -176,7 +176,7 @@ DUK_INTERNAL_DECL void duk_xdef_prop(duk_context *ctx, duk_idx_t obj_idx, duk_sm DUK_INTERNAL_DECL void duk_xdef_prop_index(duk_context *ctx, duk_idx_t obj_idx, duk_uarridx_t arr_idx, duk_small_uint_t desc_flags); /* [val] -> [] */ DUK_INTERNAL_DECL void duk_xdef_prop_stridx(duk_context *ctx, duk_idx_t obj_idx, duk_small_int_t stridx, duk_small_uint_t desc_flags); /* [val] -> [] */ DUK_INTERNAL_DECL void duk_xdef_prop_stridx_builtin(duk_context *ctx, duk_idx_t obj_idx, duk_small_int_t stridx, duk_small_int_t builtin_idx, duk_small_uint_t desc_flags); /* [] -> [] */ -DUK_INTERNAL_DECL void duk_xdef_prop_stridx_thrower(duk_context *ctx, duk_idx_t obj_idx, duk_small_int_t stridx, duk_small_uint_t desc_flags); /* [] -> [] */ +DUK_INTERNAL_DECL void duk_xdef_prop_stridx_thrower(duk_context *ctx, duk_idx_t obj_idx, duk_small_int_t stridx); /* [] -> [] */ /* These are macros for now, but could be separate functions to reduce code * footprint (check call site count before refactoring). diff --git a/src-input/duk_api_object.c b/src-input/duk_api_object.c index 37848919..15ee9904 100644 --- a/src-input/duk_api_object.c +++ b/src-input/duk_api_object.c @@ -374,11 +374,12 @@ DUK_INTERNAL void duk_xdef_prop_stridx_builtin(duk_context *ctx, duk_idx_t obj_i * object creation code, function instance creation code, and Function.prototype.bind(). */ -DUK_INTERNAL void duk_xdef_prop_stridx_thrower(duk_context *ctx, duk_idx_t obj_idx, duk_small_int_t stridx, duk_small_uint_t desc_flags) { - duk_hthread *thr = (duk_hthread *) ctx; - duk_hobject *obj = duk_require_hobject(ctx, obj_idx); - duk_hobject *thrower = thr->builtins[DUK_BIDX_TYPE_ERROR_THROWER]; - duk_hobject_define_accessor_internal(thr, obj, DUK_HTHREAD_GET_STRING(thr, stridx), thrower, thrower, desc_flags); +DUK_INTERNAL void duk_xdef_prop_stridx_thrower(duk_context *ctx, duk_idx_t obj_idx, duk_small_int_t stridx) { + obj_idx = duk_require_normalize_index(ctx, obj_idx); + duk_push_hstring_stridx(ctx, stridx); + duk_push_hobject_bidx(ctx, DUK_BIDX_TYPE_ERROR_THROWER); + duk_dup_top(ctx); + duk_def_prop(ctx, obj_idx, DUK_DEFPROP_HAVE_SETTER | DUK_DEFPROP_HAVE_GETTER | DUK_DEFPROP_FORCE); /* attributes always 0 */ } /* Object.defineProperty() equivalent C binding. */ diff --git a/src-input/duk_bi_function.c b/src-input/duk_bi_function.c index 644b8ba5..8d7e4c37 100644 --- a/src-input/duk_bi_function.c +++ b/src-input/duk_bi_function.c @@ -319,8 +319,8 @@ DUK_INTERNAL duk_ret_t duk_bi_function_prototype_bind(duk_context *ctx) { duk_xdef_prop_stridx(ctx, -2, DUK_STRIDX_LENGTH, DUK_PROPDESC_FLAGS_NONE); /* attrs in E5 Section 15.3.5.1 */ /* caller and arguments must use the same thrower, [[ThrowTypeError]] */ - duk_xdef_prop_stridx_thrower(ctx, -1, DUK_STRIDX_CALLER, DUK_PROPDESC_FLAGS_NONE); - duk_xdef_prop_stridx_thrower(ctx, -1, DUK_STRIDX_LC_ARGUMENTS, DUK_PROPDESC_FLAGS_NONE); + duk_xdef_prop_stridx_thrower(ctx, -1, DUK_STRIDX_CALLER); + duk_xdef_prop_stridx_thrower(ctx, -1, DUK_STRIDX_LC_ARGUMENTS); /* these non-standard properties are copied for convenience */ /* XXX: 'copy properties' API call? */ diff --git a/src-input/duk_hobject_props.c b/src-input/duk_hobject_props.c index 8f2181b3..09d15b09 100644 --- a/src-input/duk_hobject_props.c +++ b/src-input/duk_hobject_props.c @@ -4741,51 +4741,6 @@ DUK_INTERNAL void duk_hobject_define_property_internal_arridx(duk_hthread *thr, duk_pop(ctx); /* [ ... key ] -> [ ... ] */ } -/* - * Internal helper for defining an accessor property, ignoring - * normal semantics such as extensibility, write protection etc. - * Overwrites any existing value and attributes. This is called - * very rarely, so the implementation first sets a value to undefined - * and then changes the entry to an accessor (this is to save code space). - */ - -DUK_INTERNAL void duk_hobject_define_accessor_internal(duk_hthread *thr, duk_hobject *obj, duk_hstring *key, duk_hobject *getter, duk_hobject *setter, duk_small_uint_t propflags) { - duk_context *ctx = (duk_context *) thr; - duk_int_t e_idx; - duk_int_t h_idx; - - DUK_DDD(DUK_DDDPRINT("define new accessor (internal): thr=%p, obj=%!O, key=%!O, " - "getter=%!O, setter=%!O, flags=0x%02lx", - (void *) thr, (duk_heaphdr *) obj, (duk_heaphdr *) key, - (duk_heaphdr *) getter, (duk_heaphdr *) setter, - (unsigned long) propflags)); - - DUK_ASSERT(thr != NULL); - DUK_ASSERT(thr->heap != NULL); - DUK_ASSERT(obj != NULL); - DUK_ASSERT(key != NULL); - DUK_ASSERT((propflags & ~DUK_PROPDESC_FLAGS_MASK) == 0); - /* setter and/or getter may be NULL */ - DUK_ASSERT(!DUK_HEAPHDR_HAS_READONLY((duk_heaphdr *) obj)); - - DUK_ASSERT_VALSTACK_SPACE(thr, DUK__VALSTACK_SPACE); - - /* force the property to 'undefined' to create a slot for it */ - duk_push_undefined(ctx); - duk_hobject_define_property_internal(thr, obj, key, propflags); - duk_hobject_find_existing_entry(thr->heap, obj, key, &e_idx, &h_idx); - DUK_DDD(DUK_DDDPRINT("accessor slot: e_idx=%ld, h_idx=%ld", (long) e_idx, (long) h_idx)); - DUK_ASSERT(e_idx >= 0); - DUK_ASSERT((duk_uint32_t) e_idx < DUK_HOBJECT_GET_ENEXT(obj)); - - /* no need to decref, as previous value is 'undefined' */ - DUK_HOBJECT_E_SLOT_SET_ACCESSOR(thr->heap, obj, e_idx); - DUK_HOBJECT_E_SET_VALUE_GETTER(thr->heap, obj, e_idx, getter); - DUK_HOBJECT_E_SET_VALUE_SETTER(thr->heap, obj, e_idx, setter); - DUK_HOBJECT_INCREF_ALLOWNULL(thr, getter); - DUK_HOBJECT_INCREF_ALLOWNULL(thr, setter); -} - /* * Internal helpers for managing object 'length' */ diff --git a/src-input/duk_hthread_builtins.c b/src-input/duk_hthread_builtins.c index 27175996..356c5da8 100644 --- a/src-input/duk_hthread_builtins.c +++ b/src-input/duk_hthread_builtins.c @@ -418,7 +418,7 @@ DUK_INTERNAL void duk_hthread_create_builtin_objects(duk_hthread *thr) { num = (duk_small_uint_t) duk_bd_decode(bd, DUK__NUM_NORMAL_PROPS_BITS); DUK_DDD(DUK_DDDPRINT("built-in object %ld, %ld normal valued properties", (long) i, (long) num)); for (j = 0; j < num; j++) { - duk_small_uint_t prop_flags; + duk_small_uint_t defprop_flags; duk__push_stridx_or_string(ctx, bd); @@ -430,15 +430,27 @@ DUK_INTERNAL void duk_hthread_create_builtin_objects(duk_hthread *thr) { */ if (duk_bd_decode_flag(bd)) { - prop_flags = (duk_small_uint_t) duk_bd_decode(bd, DUK__PROP_FLAGS_BITS); + defprop_flags = (duk_small_uint_t) duk_bd_decode(bd, DUK__PROP_FLAGS_BITS); } else { - prop_flags = DUK_PROPDESC_FLAGS_WC; + defprop_flags = DUK_PROPDESC_FLAGS_WC; } + defprop_flags |= DUK_DEFPROP_FORCE | + DUK_DEFPROP_HAVE_VALUE | + DUK_DEFPROP_HAVE_WRITABLE | + DUK_DEFPROP_HAVE_ENUMERABLE | + DUK_DEFPROP_HAVE_CONFIGURABLE; /* Defaults for data properties. */ + + /* The writable, enumerable, configurable flags in prop_flags + * match both duk_def_prop() and internal property flags. + */ + DUK_ASSERT(DUK_PROPDESC_FLAG_WRITABLE == DUK_DEFPROP_WRITABLE); + DUK_ASSERT(DUK_PROPDESC_FLAG_ENUMERABLE == DUK_DEFPROP_ENUMERABLE); + DUK_ASSERT(DUK_PROPDESC_FLAG_CONFIGURABLE == DUK_DEFPROP_CONFIGURABLE); t = (duk_small_uint_t) duk_bd_decode(bd, DUK__PROP_TYPE_BITS); DUK_DDD(DUK_DDDPRINT("built-in %ld, normal-valued property %ld, key %!T, flags 0x%02lx, type %ld", - (long) i, (long) j, duk_get_tval(ctx, -1), (unsigned long) prop_flags, (long) t)); + (long) i, (long) j, duk_get_tval(ctx, -1), (unsigned long) defprop_flags, (long) t)); switch (t) { case DUK__PROP_TYPE_DOUBLE: { @@ -479,38 +491,28 @@ DUK_INTERNAL void duk_hthread_create_builtin_objects(duk_hthread *thr) { duk_c_function c_func_getter; duk_c_function c_func_setter; - /* XXX: this is a bit awkward because there is no exposed helper - * in the API style, only this internal helper. - */ DUK_DDD(DUK_DDDPRINT("built-in accessor property: objidx=%ld, key=%!T, getteridx=%ld, setteridx=%ld, flags=0x%04lx", - (long) i, duk_get_tval(ctx, -1), (long) natidx_getter, (long) natidx_setter, (unsigned long) prop_flags)); + (long) i, duk_get_tval(ctx, -1), (long) natidx_getter, (long) natidx_setter, (unsigned long) defprop_flags)); c_func_getter = duk_bi_native_functions[natidx_getter]; - c_func_setter = duk_bi_native_functions[natidx_setter]; if (c_func_getter != NULL) { duk_push_c_function_noconstruct_noexotic(ctx, c_func_getter, 0); /* always 0 args */ - } else { - duk_push_undefined(ctx); + defprop_flags |= DUK_DEFPROP_HAVE_GETTER; } + c_func_setter = duk_bi_native_functions[natidx_setter]; if (c_func_setter != NULL) { duk_push_c_function_noconstruct_noexotic(ctx, c_func_setter, 1); /* always 1 arg */ - } else { - duk_push_undefined(ctx); + defprop_flags |= DUK_DEFPROP_HAVE_SETTER; } - /* XXX: magic for getter/setter? use duk_def_prop()? */ + /* XXX: magic for getter/setter? */ - DUK_ASSERT((prop_flags & DUK_PROPDESC_FLAG_WRITABLE) == 0); /* genbuiltins.py ensures */ + /* Writable flag doesn't make sense for an accessor. */ + DUK_ASSERT((defprop_flags & DUK_PROPDESC_FLAG_WRITABLE) == 0); /* genbuiltins.py ensures */ - prop_flags |= DUK_PROPDESC_FLAG_ACCESSOR; /* accessor flag not encoded explicitly */ - duk_hobject_define_accessor_internal(thr, - duk_require_hobject(ctx, i), - duk_get_hstring(ctx, -3), - duk_get_hobject(ctx, -2), - duk_get_hobject(ctx, -1), - prop_flags); - duk_pop_3(ctx); /* key, getter and setter, now reachable through object */ - goto skip_value; + defprop_flags &= ~(DUK_DEFPROP_HAVE_VALUE | DUK_DEFPROP_HAVE_WRITABLE); + defprop_flags |= DUK_DEFPROP_HAVE_ENUMERABLE | DUK_DEFPROP_HAVE_CONFIGURABLE; + break; } default: { /* exhaustive */ @@ -518,11 +520,8 @@ DUK_INTERNAL void duk_hthread_create_builtin_objects(duk_hthread *thr) { } } - DUK_ASSERT((prop_flags & DUK_PROPDESC_FLAG_ACCESSOR) == 0); - duk_xdef_prop(ctx, i, prop_flags); - - skip_value: - continue; /* avoid empty label at the end of a compound statement */ + duk_def_prop(ctx, i, defprop_flags); + DUK_ASSERT_TOP(ctx, DUK_NUM_ALL_BUILTINS); } /* native function properties */ diff --git a/src-input/duk_js_call.c b/src-input/duk_js_call.c index 49ecd926..bbbc622e 100644 --- a/src-input/duk_js_call.c +++ b/src-input/duk_js_call.c @@ -308,8 +308,8 @@ DUK_LOCAL void duk__create_arguments_object(duk_hthread *thr, DUK_DDD(DUK_DDDPRINT("strict function, setting caller/callee to throwers")); - duk_xdef_prop_stridx_thrower(ctx, i_arg, DUK_STRIDX_CALLER, DUK_PROPDESC_FLAGS_NONE); - duk_xdef_prop_stridx_thrower(ctx, i_arg, DUK_STRIDX_CALLEE, DUK_PROPDESC_FLAGS_NONE); + duk_xdef_prop_stridx_thrower(ctx, i_arg, DUK_STRIDX_CALLER); + duk_xdef_prop_stridx_thrower(ctx, i_arg, DUK_STRIDX_CALLEE); } else { DUK_DDD(DUK_DDDPRINT("non-strict function, setting callee to actual value")); duk_push_hobject(ctx, func); diff --git a/src-input/duk_js_var.c b/src-input/duk_js_var.c index 8492980d..d04d8ba8 100644 --- a/src-input/duk_js_var.c +++ b/src-input/duk_js_var.c @@ -393,8 +393,8 @@ void duk_js_push_closure(duk_hthread *thr, /* [ ... closure template ] */ if (DUK_HOBJECT_HAS_STRICT(&fun_clos->obj)) { - duk_xdef_prop_stridx_thrower(ctx, -2, DUK_STRIDX_CALLER, DUK_PROPDESC_FLAGS_NONE); - duk_xdef_prop_stridx_thrower(ctx, -2, DUK_STRIDX_LC_ARGUMENTS, DUK_PROPDESC_FLAGS_NONE); + duk_xdef_prop_stridx_thrower(ctx, -2, DUK_STRIDX_CALLER); + duk_xdef_prop_stridx_thrower(ctx, -2, DUK_STRIDX_LC_ARGUMENTS); } else { #ifdef DUK_USE_NONSTD_FUNC_CALLER_PROPERTY DUK_DDD(DUK_DDDPRINT("function is non-strict and non-standard 'caller' property in use, add initial 'null' value")); From b332c7dd27e0522ab31808b2e385bab935d25f7f Mon Sep 17 00:00:00 2001 From: Sami Vaarala Date: Wed, 12 Oct 2016 22:42:24 +0300 Subject: [PATCH 2/2] Releases: remove internal accessor helper --- RELEASES.rst | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/RELEASES.rst b/RELEASES.rst index fc66ab48..938e0cc1 100644 --- a/RELEASES.rst +++ b/RELEASES.rst @@ -1992,9 +1992,10 @@ Planned opcode handler optimizations (GH-903); refcount optimizations (GH-443, GH-973); minor RegExp compile/execute optimizations (GH-974) -* Miscellaneous footprint improvements: RegExp compiler/executor (GH-977), - internal duk_dup() variants (GH-990), allow stripping of (almost) all - built-ins for low memory builds (GH-989) +* Miscellaneous footprint improvements: RegExp compiler/executor (GH-977); + internal duk_dup() variants (GH-990); allow stripping of (almost) all + built-ins for low memory builds (GH-989); remove internal accessor setup + helper and use duk_def_prop() instead (GH-1010) * Internal change: rework shared internal string handling so that shared strings are plain string constants used in macro values, rather than