From 72685ac7aa356af6ea381104070858ffbbfcbc91 Mon Sep 17 00:00:00 2001 From: andycall Date: Fri, 22 Nov 2024 18:06:05 +0800 Subject: [PATCH 1/3] fix: rework inline cache handing. mirror of https://github.com/quickjs-ng/quickjs/pull/609 --- .../quickjs/include/quickjs/quickjs.h | 9 +-- .../third_party/quickjs/src/core/function.c | 61 +++++++++++-------- bridge/third_party/quickjs/src/core/ic.c | 21 ++++--- bridge/third_party/quickjs/src/core/ic.h | 9 +-- bridge/third_party/quickjs/src/core/object.c | 49 +++++++-------- bridge/third_party/quickjs/src/core/types.h | 11 +++- 6 files changed, 91 insertions(+), 69 deletions(-) diff --git a/bridge/third_party/quickjs/include/quickjs/quickjs.h b/bridge/third_party/quickjs/include/quickjs/quickjs.h index f6198145e4..11659d0f3e 100644 --- a/bridge/third_party/quickjs/include/quickjs/quickjs.h +++ b/bridge/third_party/quickjs/include/quickjs/quickjs.h @@ -742,16 +742,17 @@ JSValue JS_NewArray(JSContext *ctx); int JS_IsArray(JSContext *ctx, JSValueConst val); typedef struct InlineCache InlineCache; -JSValue JS_GetPropertyInternal(JSContext* ctx, JSValueConst obj, JSAtom prop, JSValueConst receiver, InlineCache *ic, JS_BOOL throw_ref_error); -JSValue JS_GetPropertyInternalWithIC(JSContext* ctx, JSValueConst obj, JSAtom prop, JSValueConst receiver, InlineCache *ic, int32_t offset, JS_BOOL throw_ref_error); +typedef struct InlineCacheUpdate InlineCacheUpdate; +JSValue JS_GetPropertyInternal(JSContext* ctx, JSValueConst obj, JSAtom prop, JSValueConst receiver, InlineCacheUpdate *icu, JS_BOOL throw_ref_error); +JSValue JS_GetPropertyInternalWithIC(JSContext* ctx, JSValueConst obj, JSAtom prop, JSValueConst receiver, InlineCacheUpdate *icu, JS_BOOL throw_ref_error); static js_force_inline JSValue JS_GetProperty(JSContext* ctx, JSValueConst this_obj, JSAtom prop) { return JS_GetPropertyInternal(ctx, this_obj, prop, this_obj, NULL, 0); } JSValue JS_GetPropertyStr(JSContext* ctx, JSValueConst this_obj, const char* prop); JSValue JS_GetPropertyUint32(JSContext* ctx, JSValueConst this_obj, uint32_t idx); -int JS_SetPropertyInternal(JSContext* ctx, JSValueConst this_obj, JSAtom prop, JSValue val, int flags, InlineCache *ic); -int JS_SetPropertyInternalWithIC(JSContext* ctx, JSValueConst this_obj, JSAtom prop, JSValue val, int flags, InlineCache *ic, int32_t offset); +int JS_SetPropertyInternal(JSContext* ctx, JSValueConst this_obj, JSAtom prop, JSValue val, int flags, InlineCacheUpdate *icu); +int JS_SetPropertyInternalWithIC(JSContext* ctx, JSValueConst this_obj, JSAtom prop, JSValue val, int flags, InlineCacheUpdate *icu); static inline int JS_SetProperty(JSContext* ctx, JSValueConst this_obj, JSAtom prop, JSValue val) { return JS_SetPropertyInternal(ctx, this_obj, prop, val, JS_PROP_THROW, NULL); } diff --git a/bridge/third_party/quickjs/src/core/function.c b/bridge/third_party/quickjs/src/core/function.c index ae8262cc56..126d3f8b69 100644 --- a/bridge/third_party/quickjs/src/core/function.c +++ b/bridge/third_party/quickjs/src/core/function.c @@ -1479,16 +1479,17 @@ JSValue JS_CallInternal(JSContext* caller_ctx, CASE(OP_get_field) : { JSValue val; JSAtom atom; + InlineCacheUpdate icu; atom = get_u32(pc); pc += 4; - - val = JS_GetPropertyInternal(ctx, sp[-1], atom, sp[-1], ic, FALSE); + + icu = (InlineCacheUpdate){ic, INLINE_CACHE_MISS}; + val = JS_GetPropertyInternal(ctx, sp[-1], atom, sp[-1], &icu, FALSE); if (unlikely(JS_IsException(val))) goto exception; - if (ic != NULL && ic->updated == TRUE) { - ic->updated = FALSE; + if (icu.offset != INLINE_CACHE_MISS) { put_u8(pc - 5, OP_get_field_ic); - put_u32(pc - 4, ic->updated_offset); + put_u32(pc - 4, icu.offset); // safe free call because ic struct will retain atom JS_FreeAtom(ctx, atom); } @@ -1500,15 +1501,16 @@ JSValue JS_CallInternal(JSContext* caller_ctx, CASE(OP_get_field_ic): { JSValue val; JSAtom atom; - int32_t ic_offset; + uint32_t ic_offset; + InlineCacheUpdate icu; ic_offset = get_u32(pc); atom = get_ic_atom(ic, ic_offset); pc += 4; - - val = JS_GetPropertyInternalWithIC(ctx, sp[-1], atom, sp[-1], ic, ic_offset, FALSE); - ic->updated = FALSE; + icu = (InlineCacheUpdate){ic, ic_offset}; + val = JS_GetPropertyInternalWithIC(ctx, sp[-1], atom, sp[-1], &icu, FALSE); if (unlikely(JS_IsException(val))) goto exception; + assert(icu.offset == ic_offset); JS_FreeValue(ctx, sp[-1]); sp[-1] = val; } @@ -1517,16 +1519,19 @@ JSValue JS_CallInternal(JSContext* caller_ctx, CASE(OP_get_field2) : { JSValue val; JSAtom atom; + InlineCacheUpdate icu; atom = get_u32(pc); pc += 4; - val = JS_GetPropertyInternal(ctx, sp[-1], atom, sp[-1], ic, FALSE); + icu = (InlineCacheUpdate){ic, INLINE_CACHE_MISS}; + + val = JS_GetPropertyInternal(ctx, sp[-1], atom, sp[-1], &icu, FALSE); if (unlikely(JS_IsException(val))) goto exception; - if (ic != NULL && ic->updated == TRUE) { - ic->updated = FALSE; + if (icu.offset != INLINE_CACHE_MISS) { + put_u8(pc - 5, OP_get_field2_ic); - put_u32(pc - 4, ic->updated_offset); + put_u32(pc - 4, icu.offset); // safe free call because ic struct will retain atom JS_FreeAtom(ctx, atom); } @@ -1537,15 +1542,18 @@ JSValue JS_CallInternal(JSContext* caller_ctx, CASE(OP_get_field2_ic): { JSValue val; JSAtom atom; - int32_t ic_offset; + uint32_t ic_offset; + InlineCacheUpdate icu; ic_offset = get_u32(pc); atom = get_ic_atom(ic, ic_offset); pc += 4; - - val = JS_GetPropertyInternalWithIC(ctx, sp[-1], atom, sp[-1], ic, ic_offset, FALSE); - ic->updated = FALSE; + + icu = (InlineCacheUpdate){ic, ic_offset}; + val = JS_GetPropertyInternalWithIC(ctx, sp[-1], atom, sp[-1], &icu, FALSE); + if (unlikely(JS_IsException(val))) goto exception; + assert(icu.offset == ic_offset); *sp++ = val; } BREAK; @@ -1553,18 +1561,19 @@ JSValue JS_CallInternal(JSContext* caller_ctx, CASE(OP_put_field) : { int ret; JSAtom atom; + InlineCacheUpdate icu; atom = get_u32(pc); pc += 4; - ret = JS_SetPropertyInternal(ctx, sp[-2], atom, sp[-1], JS_PROP_THROW_STRICT, ic); + icu = (InlineCacheUpdate){ic, INLINE_CACHE_MISS}; + ret = JS_SetPropertyInternal(ctx, sp[-2], atom, sp[-1], JS_PROP_THROW_STRICT, &icu); JS_FreeValue(ctx, sp[-2]); sp -= 2; if (unlikely(ret < 0)) goto exception; - if (ic != NULL && ic->updated == TRUE) { - ic->updated = FALSE; + if (icu.offset != INLINE_CACHE_MISS) { put_u8(pc - 5, OP_put_field_ic); - put_u32(pc - 4, ic->updated_offset); + put_u32(pc - 4, icu.offset); // safe free call because ic struct will retain atom JS_FreeAtom(ctx, atom); } @@ -1574,17 +1583,19 @@ JSValue JS_CallInternal(JSContext* caller_ctx, CASE(OP_put_field_ic): { int ret; JSAtom atom; - int32_t ic_offset; + uint32_t ic_offset; + InlineCacheUpdate icu; ic_offset = get_u32(pc); atom = get_ic_atom(ic, ic_offset); pc += 4; - - ret = JS_SetPropertyInternalWithIC(ctx, sp[-2], atom, sp[-1], JS_PROP_THROW_STRICT, ic, ic_offset); - ic->updated = FALSE; + icu = (InlineCacheUpdate){ic, ic_offset}; + ret = JS_SetPropertyInternalWithIC(ctx, sp[-2], atom, sp[-1], + JS_PROP_THROW_STRICT, &icu); JS_FreeValue(ctx, sp[-2]); sp -= 2; if (unlikely(ret < 0)) goto exception; + assert(icu.offset == ic_offset); } BREAK; diff --git a/bridge/third_party/quickjs/src/core/ic.c b/bridge/third_party/quickjs/src/core/ic.c index d60551c9c5..17e5d9bddf 100644 --- a/bridge/third_party/quickjs/src/core/ic.c +++ b/bridge/third_party/quickjs/src/core/ic.c @@ -43,8 +43,6 @@ InlineCache *init_ic(JSContext *ctx) { goto fail; memset(ic->hash, 0, sizeof(ic->hash[0]) * ic->capacity); ic->cache = NULL; - ic->updated = FALSE; - ic->updated_offset = 0; return ic; fail: return NULL; @@ -137,11 +135,11 @@ int free_ic(InlineCache *ic) { } #if _MSC_VER -uint32_t add_ic_slot(InlineCache *ic, JSAtom atom, JSObject *object, +uint32_t add_ic_slot(InlineCacheUpdate *icu, JSAtom atom, JSObject *object, uint32_t prop_offset, JSObject* prototype) #else -force_inline uint32_t add_ic_slot(InlineCache *ic, JSAtom atom, JSObject *object, - uint32_t prop_offset, JSObject* prototype) +force_inline void add_ic_slot(InlineCacheUpdate *icu, JSAtom atom, JSObject *object, + uint32_t prop_offset, JSObject* prototype) #endif { int32_t i; @@ -150,12 +148,19 @@ force_inline uint32_t add_ic_slot(InlineCache *ic, JSAtom atom, JSObject *object InlineCacheRingSlot *cr; InlineCacheRingItem *ci; JSRuntime* rt; + InlineCache *ic; JSShape *sh; - JSObject *proto; + + if (!icu) + return; + ic = icu->ic; + + if (!ic) + return; + cr = NULL; rt = ic->ctx->rt; sh = NULL; - proto = NULL; h = get_index_hash(atom, ic->hash_bits); for (ch = ic->hash[h]; ch != NULL; ch = ch->next) if (ch->atom == atom) { @@ -197,7 +202,7 @@ force_inline uint32_t add_ic_slot(InlineCache *ic, JSAtom atom, JSObject *object ic_watchpoint_free_handler); } end: - return ch->index; + icu->offset = ch->index; } #if _MSC_VER diff --git a/bridge/third_party/quickjs/src/core/ic.h b/bridge/third_party/quickjs/src/core/ic.h index 4dcd3a2b76..882c06f3ae 100644 --- a/bridge/third_party/quickjs/src/core/ic.h +++ b/bridge/third_party/quickjs/src/core/ic.h @@ -34,12 +34,13 @@ InlineCache *init_ic(JSContext *ctx); int rebuild_ic(InlineCache *ic); int resize_ic_hash(InlineCache *ic); int free_ic(InlineCache *ic); -uint32_t add_ic_slot(InlineCache *ic, JSAtom atom, JSObject *object, +void add_ic_slot(InlineCacheUpdate *icu, JSAtom atom, JSObject *object, uint32_t prop_offset, JSObject* prototype); uint32_t add_ic_slot1(InlineCache *ic, JSAtom atom); -force_inline int32_t get_ic_prop_offset(InlineCache *ic, uint32_t cache_offset, +force_inline uint32_t get_ic_prop_offset(const InlineCacheUpdate *icu, JSShape *shape, JSObject **prototype) { - uint32_t i; + uint32_t i, cache_offset = icu->offset; + InlineCache *ic = icu->ic; InlineCacheRingSlot *cr; InlineCacheRingItem *buffer; assert(cache_offset < ic->capacity); @@ -60,7 +61,7 @@ force_inline int32_t get_ic_prop_offset(InlineCache *ic, uint32_t cache_offset, } *prototype = NULL; - return -1; + return INLINE_CACHE_MISS; } force_inline JSAtom get_ic_atom(InlineCache *ic, uint32_t cache_offset) { diff --git a/bridge/third_party/quickjs/src/core/object.c b/bridge/third_party/quickjs/src/core/object.c index 8a5dbfe623..4079e3d989 100644 --- a/bridge/third_party/quickjs/src/core/object.c +++ b/bridge/third_party/quickjs/src/core/object.c @@ -386,9 +386,10 @@ static int JS_AutoInitProperty(JSContext *ctx, JSObject *p, JSAtom prop, JSValue JS_GetPropertyInternal(JSContext *ctx, JSValueConst obj, JSAtom prop, JSValueConst this_obj, - InlineCache *ic, BOOL throw_ref_error) + InlineCacheUpdate *icu, + BOOL throw_ref_error) { - JSObject *p, *p1; + JSObject *p; JSProperty *pr; JSShapeProperty *prs; uint32_t tag, offset, proto_depth; @@ -432,7 +433,6 @@ JSValue JS_GetPropertyInternal(JSContext *ctx, JSValueConst obj, p = JS_VALUE_GET_OBJ(obj); } - p1 = p; for(;;) { prs = find_own_property_ic(&pr, p, prop, &offset); if (prs) { @@ -459,10 +459,9 @@ JSValue JS_GetPropertyInternal(JSContext *ctx, JSValueConst obj, continue; } } else { - // basic poly ic is only used for fast path - if (ic && p1->shape->is_hashed && p->shape->is_hashed) { - ic->updated = TRUE; - ic->updated_offset = add_ic_slot(ic, prop, p1, offset, proto_depth > 0 ? p : NULL); + // basic poly ic and prototype ic is only used for fast path + if (icu && proto_depth == 0 && p->shape->is_hashed) { + add_ic_slot(icu, prop, p, offset, proto_depth > 0 ? p : NULL); } return JS_DupValue(ctx, pr->u.value); } @@ -541,29 +540,29 @@ JSValue JS_GetPropertyInternal(JSContext *ctx, JSValueConst obj, #if _MSC_VER JSValue JS_GetPropertyInternalWithIC(JSContext *ctx, JSValueConst obj, JSAtom prop, JSValueConst this_obj, - InlineCache *ic, int32_t offset, + InlineCacheUpdate *icu, BOOL throw_ref_error) #else force_inline JSValue JS_GetPropertyInternalWithIC(JSContext *ctx, JSValueConst obj, JSAtom prop, JSValueConst this_obj, - InlineCache *ic, int32_t offset, + InlineCacheUpdate *icu, BOOL throw_ref_error) #endif { - uint32_t tag; + uint32_t tag, offset; JSObject *p, *proto; tag = JS_VALUE_GET_TAG(obj); if (unlikely(tag != JS_TAG_OBJECT)) goto slow_path; p = JS_VALUE_GET_OBJ(obj); - offset = get_ic_prop_offset(ic, offset, p->shape, &proto); - if (likely(offset >= 0)) { + offset = get_ic_prop_offset(icu, p->shape, &proto); + if (likely(offset != INLINE_CACHE_MISS)) { if (proto) p = proto; return JS_DupValue(ctx, p->prop[offset].u.value); } slow_path: - return JS_GetPropertyInternal(ctx, obj, prop, this_obj, ic, throw_ref_error); + return JS_GetPropertyInternal(ctx, obj, prop, this_obj, icu, throw_ref_error); } JSValue JS_GetOwnPropertyNames2(JSContext* ctx, JSValueConst obj1, int flags, int kind) { @@ -1774,7 +1773,7 @@ int JS_SetPropertyGeneric(JSContext* ctx, JSValueConst obj, JSAtom prop, JSValue freed by the function. 'flags' is a bitmask of JS_PROP_NO_ADD, JS_PROP_THROW or JS_PROP_THROW_STRICT. If JS_PROP_NO_ADD is set, the new property is not added and an error is raised. */ -int JS_SetPropertyInternal(JSContext* ctx, JSValueConst this_obj, JSAtom prop, JSValue val, int flags, InlineCache *ic) { +int JS_SetPropertyInternal(JSContext* ctx, JSValueConst this_obj, JSAtom prop, JSValue val, int flags, InlineCacheUpdate *icu) { JSObject *p, *p1; JSShapeProperty* prs; JSProperty* pr; @@ -1810,9 +1809,8 @@ int JS_SetPropertyInternal(JSContext* ctx, JSValueConst this_obj, JSAtom prop, J if (prs) { if (likely((prs->flags & (JS_PROP_TMASK | JS_PROP_WRITABLE | JS_PROP_LENGTH)) == JS_PROP_WRITABLE)) { /* fast case */ - if (ic && p->shape->is_hashed) { - ic->updated = TRUE; - ic->updated_offset = add_ic_slot(ic, prop, p, offset, NULL); + if (icu && p->shape->is_hashed) { + add_ic_slot(icu, prop, p, offset, NULL); } set_value(ctx, &pr->u.value, val); return TRUE; @@ -1984,33 +1982,32 @@ int JS_SetPropertyInternal(JSContext* ctx, JSValueConst this_obj, JSAtom prop, J } pr->u.value = val; /* fast case */ - if (ic && p->shape->is_hashed) { - ic->updated = TRUE; - ic->updated_offset = add_ic_slot(ic, prop, p, p->shape->prop_count - 1, NULL); + if (icu && icu->ic && p->shape->is_hashed) { + add_ic_slot(icu, prop, p, p->shape->prop_count - 1, NULL); } return TRUE; } #if _MSC_VER -int JS_SetPropertyInternalWithIC(JSContext* ctx, JSValueConst this_obj, JSAtom prop, JSValue val, int flags, InlineCache *ic, int32_t offset) { +int JS_SetPropertyInternalWithIC(JSContext* ctx, JSValueConst this_obj, JSAtom prop, JSValue val, int flags, InlineCacheUpdate *icu) { #else -force_inline int JS_SetPropertyInternalWithIC(JSContext* ctx, JSValueConst this_obj, JSAtom prop, JSValue val, int flags, InlineCache *ic, int32_t offset) { +force_inline int JS_SetPropertyInternalWithIC(JSContext* ctx, JSValueConst this_obj, JSAtom prop, JSValue val, int flags, InlineCacheUpdate *icu) { #endif - uint32_t tag; + uint32_t tag, offset; JSObject *p, *proto; tag = JS_VALUE_GET_TAG(this_obj); if (unlikely(tag != JS_TAG_OBJECT)) goto slow_path; p = JS_VALUE_GET_OBJ(this_obj); - offset = get_ic_prop_offset(ic, offset, p->shape, &proto); - if (likely(offset >= 0)) { + offset = get_ic_prop_offset(icu, p->shape, &proto); + if (likely(offset != INLINE_CACHE_MISS)) { if (proto) goto slow_path; set_value(ctx, &p->prop[offset].u.value, val); return TRUE; } slow_path: - return JS_SetPropertyInternal(ctx, this_obj, prop, val, flags, ic); + return JS_SetPropertyInternal(ctx, this_obj, prop, val, flags, icu); } /* flags can be JS_PROP_THROW or JS_PROP_THROW_STRICT */ diff --git a/bridge/third_party/quickjs/src/core/types.h b/bridge/third_party/quickjs/src/core/types.h index 4b5bb389ce..762be45dc5 100644 --- a/bridge/third_party/quickjs/src/core/types.h +++ b/bridge/third_party/quickjs/src/core/types.h @@ -562,10 +562,17 @@ typedef struct InlineCache { JSContext* ctx; InlineCacheHashSlot **hash; InlineCacheRingSlot *cache; - uint32_t updated_offset; - BOOL updated; } InlineCache; +#define INLINE_CACHE_MISS ((uint32_t)-1) // sentinel +// This is a struct so we don't tie up two argument registers in calls to +// JS_GetPropertyInternal and JS_SetPropertyInternal in the common case +// where there is no IC and therefore no offset to update. +typedef struct InlineCacheUpdate { + InlineCache *ic; + uint32_t offset; +} InlineCacheUpdate; + typedef struct JSFunctionBytecode { JSGCObjectHeader header; /* must come first */ uint8_t js_mode; From 99257be120be5f96737508606f3b20dba9790f59 Mon Sep 17 00:00:00 2001 From: andycall Date: Fri, 22 Nov 2024 18:25:30 +0800 Subject: [PATCH 2/3] fix: fix prototype ic. --- bridge/third_party/quickjs/src/core/object.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/bridge/third_party/quickjs/src/core/object.c b/bridge/third_party/quickjs/src/core/object.c index 4079e3d989..32c5385a21 100644 --- a/bridge/third_party/quickjs/src/core/object.c +++ b/bridge/third_party/quickjs/src/core/object.c @@ -389,7 +389,7 @@ JSValue JS_GetPropertyInternal(JSContext *ctx, JSValueConst obj, InlineCacheUpdate *icu, BOOL throw_ref_error) { - JSObject *p; + JSObject *p, *p1; JSProperty *pr; JSShapeProperty *prs; uint32_t tag, offset, proto_depth; @@ -433,6 +433,7 @@ JSValue JS_GetPropertyInternal(JSContext *ctx, JSValueConst obj, p = JS_VALUE_GET_OBJ(obj); } + p1 = p; for(;;) { prs = find_own_property_ic(&pr, p, prop, &offset); if (prs) { @@ -459,9 +460,9 @@ JSValue JS_GetPropertyInternal(JSContext *ctx, JSValueConst obj, continue; } } else { - // basic poly ic and prototype ic is only used for fast path - if (icu && proto_depth == 0 && p->shape->is_hashed) { - add_ic_slot(icu, prop, p, offset, proto_depth > 0 ? p : NULL); + // basic poly ic is only used for fast path + if (p1->shape->is_hashed && p->shape->is_hashed) { + add_ic_slot(icu, prop, p1, offset, proto_depth > 0 ? p : NULL); } return JS_DupValue(ctx, pr->u.value); } From f1895bac3565780da75b700994212b60eae382e6 Mon Sep 17 00:00:00 2001 From: andycall Date: Fri, 22 Nov 2024 18:46:06 +0800 Subject: [PATCH 3/3] fix: fix windows build. --- bridge/third_party/quickjs/src/core/ic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bridge/third_party/quickjs/src/core/ic.c b/bridge/third_party/quickjs/src/core/ic.c index 17e5d9bddf..d8ac75b5e4 100644 --- a/bridge/third_party/quickjs/src/core/ic.c +++ b/bridge/third_party/quickjs/src/core/ic.c @@ -135,7 +135,7 @@ int free_ic(InlineCache *ic) { } #if _MSC_VER -uint32_t add_ic_slot(InlineCacheUpdate *icu, JSAtom atom, JSObject *object, +void add_ic_slot(InlineCacheUpdate *icu, JSAtom atom, JSObject *object, uint32_t prop_offset, JSObject* prototype) #else force_inline void add_ic_slot(InlineCacheUpdate *icu, JSAtom atom, JSObject *object,