Skip to content

Commit 763076d

Browse files
bnoordhuisbnoordhuis
authored
Rework inline cache handling (#609)
Don't store the update flag in the IC because that's a) an out-of-band signalling mechanism, and b) makes JSInlineCache bigger than it needs to be. One is allocated per function so it adds up. Another reason for making this change is that it makes visible what I strongly suspect are bugs in the original implementation.
1 parent 8cd59bf commit 763076d

File tree

1 file changed

+73
-54
lines changed

1 file changed

+73
-54
lines changed

‎quickjs.c

+73-54
Original file line numberDiff line numberDiff line change
@@ -602,21 +602,30 @@ typedef struct JSInlineCache {
602602
uint32_t hash_bits;
603603
JSInlineCacheHashSlot **hash;
604604
JSInlineCacheRingSlot *cache;
605-
uint32_t updated_offset;
606-
BOOL updated;
607605
} JSInlineCache;
608606

607+
#define INLINE_CACHE_MISS ((uint32_t)-1) // sentinel
608+
609+
// This is a struct so we don't tie up two argument registers in calls to
610+
// JS_GetPropertyInternal2 and JS_SetPropertyInternal2 in the common case
611+
// where there is no IC and therefore no offset to update.
612+
typedef struct JSInlineCacheUpdate {
613+
JSInlineCache *ic;
614+
uint32_t offset;
615+
} JSInlineCacheUpdate;
616+
609617
static JSInlineCache *init_ic(JSContext *ctx);
610618
static int rebuild_ic(JSContext *ctx, JSInlineCache *ic);
611619
static int resize_ic_hash(JSContext *ctx, JSInlineCache *ic);
612620
static int free_ic(JSRuntime *rt, JSInlineCache *ic);
613-
static uint32_t add_ic_slot(JSContext *ctx, JSInlineCache *ic, JSAtom atom, JSObject *object,
614-
uint32_t prop_offset);
621+
static void add_ic_slot(JSContext *ctx, JSInlineCacheUpdate *icu,
622+
JSAtom atom, JSObject *object, uint32_t prop_offset);
615623

616-
static int32_t get_ic_prop_offset(JSInlineCache *ic, uint32_t cache_offset,
617-
JSShape *shape)
624+
static uint32_t get_ic_prop_offset(const JSInlineCacheUpdate *icu,
625+
JSShape *shape)
618626
{
619-
uint32_t i;
627+
uint32_t i, cache_offset = icu->offset;
628+
JSInlineCache *ic = icu->ic;
620629
JSInlineCacheRingSlot *cr;
621630
JSShape *shape_slot;
622631
assert(cache_offset < ic->capacity);
@@ -635,7 +644,7 @@ static int32_t get_ic_prop_offset(JSInlineCache *ic, uint32_t cache_offset,
635644
}
636645
}
637646

638-
return -1;
647+
return INLINE_CACHE_MISS;
639648
}
640649

641650
static force_inline JSAtom get_ic_atom(JSInlineCache *ic, uint32_t cache_offset)
@@ -7283,7 +7292,8 @@ static int JS_AutoInitProperty(JSContext *ctx, JSObject *p, JSAtom prop,
72837292

72847293
static JSValue JS_GetPropertyInternal2(JSContext *ctx, JSValue obj,
72857294
JSAtom prop, JSValue this_obj,
7286-
JSInlineCache* ic, BOOL throw_ref_error)
7295+
JSInlineCacheUpdate *icu,
7296+
BOOL throw_ref_error)
72877297
{
72887298
JSObject *p;
72897299
JSProperty *pr;
@@ -7352,9 +7362,8 @@ static JSValue JS_GetPropertyInternal2(JSContext *ctx, JSValue obj,
73527362
continue;
73537363
}
73547364
} else {
7355-
if (ic && proto_depth == 0 && p->shape->is_hashed) {
7356-
ic->updated = TRUE;
7357-
ic->updated_offset = add_ic_slot(ctx, ic, prop, p, offset);
7365+
if (icu && proto_depth == 0 && p->shape->is_hashed) {
7366+
add_ic_slot(ctx, icu, prop, p, offset);
73587367
}
73597368
return js_dup(pr->u.value);
73607369
}
@@ -7444,20 +7453,20 @@ JSValue JS_GetProperty(JSContext *ctx, JSValue this_obj, JSAtom prop)
74447453

74457454
static JSValue JS_GetPropertyInternalWithIC(JSContext *ctx, JSValue obj,
74467455
JSAtom prop, JSValue this_obj,
7447-
JSInlineCache *ic, int32_t offset,
7456+
JSInlineCacheUpdate *icu,
74487457
BOOL throw_ref_error)
74497458
{
7450-
uint32_t tag;
7459+
uint32_t tag, offset;
74517460
JSObject *p;
74527461
tag = JS_VALUE_GET_TAG(obj);
74537462
if (unlikely(tag != JS_TAG_OBJECT))
74547463
goto slow_path;
74557464
p = JS_VALUE_GET_OBJ(obj);
7456-
offset = get_ic_prop_offset(ic, offset, p->shape);
7457-
if (likely(offset >= 0))
7465+
offset = get_ic_prop_offset(icu, p->shape);
7466+
if (likely(offset != INLINE_CACHE_MISS))
74587467
return js_dup(p->prop[offset].u.value);
74597468
slow_path:
7460-
return JS_GetPropertyInternal2(ctx, obj, prop, this_obj, ic, throw_ref_error);
7469+
return JS_GetPropertyInternal2(ctx, obj, prop, this_obj, icu, throw_ref_error);
74617470
}
74627471

74637472
static JSValue JS_ThrowTypeErrorPrivateNotFound(JSContext *ctx, JSAtom atom)
@@ -8611,9 +8620,9 @@ static void js_free_desc(JSContext *ctx, JSPropertyDescriptor *desc)
86118620
the new property is not added and an error is raised.
86128621
'obj' must be an object when obj != this_obj.
86138622
*/
8614-
static int JS_SetPropertyInternal2(JSContext *ctx, JSValue obj,
8615-
JSAtom prop, JSValue val, JSValue this_obj,
8616-
int flags, JSInlineCache *ic)
8623+
static int JS_SetPropertyInternal2(JSContext *ctx, JSValue obj, JSAtom prop,
8624+
JSValue val, JSValue this_obj, int flags,
8625+
JSInlineCacheUpdate *icu)
86178626
{
86188627
JSObject *p, *p1;
86198628
JSShapeProperty *prs;
@@ -8649,9 +8658,8 @@ static int JS_SetPropertyInternal2(JSContext *ctx, JSValue obj,
86498658
if (likely((prs->flags & (JS_PROP_TMASK | JS_PROP_WRITABLE |
86508659
JS_PROP_LENGTH)) == JS_PROP_WRITABLE)) {
86518660
/* fast case */
8652-
if (ic && p->shape->is_hashed) {
8653-
ic->updated = TRUE;
8654-
ic->updated_offset = add_ic_slot(ctx, ic, prop, p, offset);
8661+
if (icu && p->shape->is_hashed) {
8662+
add_ic_slot(ctx, icu, prop, p, offset);
86558663
}
86568664
set_value(ctx, &pr->u.value, val);
86578665
return TRUE;
@@ -8876,23 +8884,24 @@ int JS_SetProperty(JSContext *ctx, JSValue this_obj, JSAtom prop, JSValue val)
88768884
return JS_SetPropertyInternal2(ctx, this_obj, prop, val, this_obj, JS_PROP_THROW, NULL);
88778885
}
88788886

8887+
// XXX(bnoordhuis) only used by OP_put_field_ic, maybe inline at call site
88798888
static int JS_SetPropertyInternalWithIC(JSContext *ctx, JSValue this_obj,
88808889
JSAtom prop, JSValue val, int flags,
8881-
JSInlineCache *ic, int32_t offset) {
8882-
uint32_t tag;
8890+
JSInlineCacheUpdate *icu) {
8891+
uint32_t tag, offset;
88838892
JSObject *p;
88848893
tag = JS_VALUE_GET_TAG(this_obj);
88858894
if (unlikely(tag != JS_TAG_OBJECT))
88868895
goto slow_path;
88878896
p = JS_VALUE_GET_OBJ(this_obj);
8888-
offset = get_ic_prop_offset(ic, offset, p->shape);
8889-
if (likely(offset >= 0)) {
8897+
offset = get_ic_prop_offset(icu, p->shape);
8898+
if (likely(offset != INLINE_CACHE_MISS)) {
88908899
set_value(ctx, &p->prop[offset].u.value, val);
88918900
return TRUE;
88928901
}
88938902
slow_path:
88948903
return JS_SetPropertyInternal2(ctx, this_obj, prop, val, this_obj,
8895-
flags, ic);
8904+
flags, icu);
88968905
}
88978906

88988907
/* flags can be JS_PROP_THROW or JS_PROP_THROW_STRICT */
@@ -16129,16 +16138,17 @@ static JSValue JS_CallInternal(JSContext *caller_ctx, JSValue func_obj,
1612916138
{
1613016139
JSValue val;
1613116140
JSAtom atom;
16141+
JSInlineCacheUpdate icu;
1613216142
atom = get_u32(pc);
1613316143
pc += 4;
1613416144
sf->cur_pc = pc;
16135-
val = JS_GetPropertyInternal2(ctx, sp[-1], atom, sp[-1], ic, FALSE);
16145+
icu = (JSInlineCacheUpdate){ic, INLINE_CACHE_MISS};
16146+
val = JS_GetPropertyInternal2(ctx, sp[-1], atom, sp[-1], &icu, FALSE);
1613616147
if (unlikely(JS_IsException(val)))
1613716148
goto exception;
16138-
if (ic && ic->updated == TRUE) {
16139-
ic->updated = FALSE;
16149+
if (icu.offset != INLINE_CACHE_MISS) {
1614016150
put_u8(pc - 5, OP_get_field_ic);
16141-
put_u32(pc - 4, ic->updated_offset);
16151+
put_u32(pc - 4, icu.offset);
1614216152
JS_FreeAtom(ctx, atom);
1614316153
}
1614416154
JS_FreeValue(ctx, sp[-1]);
@@ -16150,33 +16160,37 @@ static JSValue JS_CallInternal(JSContext *caller_ctx, JSValue func_obj,
1615016160
{
1615116161
JSValue val;
1615216162
JSAtom atom;
16153-
int32_t ic_offset;
16163+
uint32_t ic_offset;
16164+
JSInlineCacheUpdate icu;
1615416165
ic_offset = get_u32(pc);
1615516166
atom = get_ic_atom(ic, ic_offset);
1615616167
pc += 4;
1615716168
sf->cur_pc = pc;
16158-
val = JS_GetPropertyInternalWithIC(ctx, sp[-1], atom, sp[-1], ic, ic_offset, FALSE);
16159-
ic->updated = FALSE;
16169+
icu = (JSInlineCacheUpdate){ic, ic_offset};
16170+
val = JS_GetPropertyInternalWithIC(ctx, sp[-1], atom, sp[-1], &icu, FALSE);
1616016171
if (unlikely(JS_IsException(val)))
1616116172
goto exception;
16173+
assert(icu.offset == ic_offset);
1616216174
JS_FreeValue(ctx, sp[-1]);
1616316175
sp[-1] = val;
1616416176
}
1616516177
BREAK;
16178+
1616616179
CASE(OP_get_field2):
1616716180
{
1616816181
JSValue val;
1616916182
JSAtom atom;
16183+
JSInlineCacheUpdate icu;
1617016184
atom = get_u32(pc);
1617116185
pc += 4;
1617216186
sf->cur_pc = pc;
16173-
val = JS_GetPropertyInternal2(ctx, sp[-1], atom, sp[-1], NULL, FALSE);
16187+
icu = (JSInlineCacheUpdate){ic, INLINE_CACHE_MISS};
16188+
val = JS_GetPropertyInternal2(ctx, sp[-1], atom, sp[-1], &icu, FALSE);
1617416189
if (unlikely(JS_IsException(val)))
1617516190
goto exception;
16176-
if (ic != NULL && ic->updated == TRUE) {
16177-
ic->updated = FALSE;
16191+
if (icu.offset != INLINE_CACHE_MISS) {
1617816192
put_u8(pc - 5, OP_get_field2_ic);
16179-
put_u32(pc - 4, ic->updated_offset);
16193+
put_u32(pc - 4, icu.offset);
1618016194
JS_FreeAtom(ctx, atom);
1618116195
}
1618216196
*sp++ = val;
@@ -16187,15 +16201,17 @@ static JSValue JS_CallInternal(JSContext *caller_ctx, JSValue func_obj,
1618716201
{
1618816202
JSValue val;
1618916203
JSAtom atom;
16190-
int32_t ic_offset;
16204+
uint32_t ic_offset;
16205+
JSInlineCacheUpdate icu;
1619116206
ic_offset = get_u32(pc);
1619216207
atom = get_ic_atom(ic, ic_offset);
1619316208
pc += 4;
1619416209
sf->cur_pc = pc;
16195-
val = JS_GetPropertyInternalWithIC(ctx, sp[-1], atom, sp[-1], ic, ic_offset, FALSE);
16196-
ic->updated = FALSE;
16210+
icu = (JSInlineCacheUpdate){ic, ic_offset};
16211+
val = JS_GetPropertyInternalWithIC(ctx, sp[-1], atom, sp[-1], &icu, FALSE);
1619716212
if (unlikely(JS_IsException(val)))
1619816213
goto exception;
16214+
assert(icu.offset == ic_offset);
1619916215
*sp++ = val;
1620016216
}
1620116217
BREAK;
@@ -16204,21 +16220,22 @@ static JSValue JS_CallInternal(JSContext *caller_ctx, JSValue func_obj,
1620416220
{
1620516221
int ret;
1620616222
JSAtom atom;
16223+
JSInlineCacheUpdate icu;
1620716224
atom = get_u32(pc);
1620816225
pc += 4;
1620916226
sf->cur_pc = pc;
16227+
icu = (JSInlineCacheUpdate){ic, INLINE_CACHE_MISS};
1621016228
ret = JS_SetPropertyInternal2(ctx,
1621116229
sp[-2], atom,
1621216230
sp[-1], sp[-2],
16213-
JS_PROP_THROW_STRICT, ic);
16231+
JS_PROP_THROW_STRICT, &icu);
1621416232
JS_FreeValue(ctx, sp[-2]);
1621516233
sp -= 2;
1621616234
if (unlikely(ret < 0))
1621716235
goto exception;
16218-
if (ic != NULL && ic->updated == TRUE) {
16219-
ic->updated = FALSE;
16236+
if (icu.offset != INLINE_CACHE_MISS) {
1622016237
put_u8(pc - 5, OP_put_field_ic);
16221-
put_u32(pc - 4, ic->updated_offset);
16238+
put_u32(pc - 4, icu.offset);
1622216239
JS_FreeAtom(ctx, atom);
1622316240
}
1622416241
}
@@ -16228,17 +16245,20 @@ static JSValue JS_CallInternal(JSContext *caller_ctx, JSValue func_obj,
1622816245
{
1622916246
int ret;
1623016247
JSAtom atom;
16231-
int32_t ic_offset;
16248+
uint32_t ic_offset;
16249+
JSInlineCacheUpdate icu;
1623216250
ic_offset = get_u32(pc);
1623316251
atom = get_ic_atom(ic, ic_offset);
1623416252
pc += 4;
1623516253
sf->cur_pc = pc;
16236-
ret = JS_SetPropertyInternalWithIC(ctx, sp[-2], atom, sp[-1], JS_PROP_THROW_STRICT, ic, ic_offset);
16237-
ic->updated = FALSE;
16254+
icu = (JSInlineCacheUpdate){ic, ic_offset};
16255+
ret = JS_SetPropertyInternalWithIC(ctx, sp[-2], atom, sp[-1],
16256+
JS_PROP_THROW_STRICT, &icu);
1623816257
JS_FreeValue(ctx, sp[-2]);
1623916258
sp -= 2;
1624016259
if (unlikely(ret < 0))
1624116260
goto exception;
16261+
assert(icu.offset == ic_offset);
1624216262
}
1624316263
BREAK;
1624416264

@@ -54406,8 +54426,6 @@ JSInlineCache *init_ic(JSContext *ctx)
5440654426
if (unlikely(!ic->hash))
5440754427
goto fail;
5440854428
ic->cache = NULL;
54409-
ic->updated = FALSE;
54410-
ic->updated_offset = 0;
5441154429
return ic;
5441254430
fail:
5441354431
js_free(ctx, ic);
@@ -54490,11 +54508,12 @@ int free_ic(JSRuntime* rt, JSInlineCache *ic)
5449054508
return 0;
5449154509
}
5449254510

54493-
uint32_t add_ic_slot(JSContext *ctx, JSInlineCache *ic, JSAtom atom, JSObject *object,
54494-
uint32_t prop_offset)
54511+
static void add_ic_slot(JSContext *ctx, JSInlineCacheUpdate *icu,
54512+
JSAtom atom, JSObject *object, uint32_t prop_offset)
5449554513
{
5449654514
int32_t i;
5449754515
uint32_t h;
54516+
JSInlineCache *ic = icu->ic;
5449854517
JSInlineCacheHashSlot *ch;
5449954518
JSInlineCacheRingSlot *cr;
5450054519
JSShape *sh;
@@ -54523,7 +54542,7 @@ uint32_t add_ic_slot(JSContext *ctx, JSInlineCache *ic, JSAtom atom, JSObject *o
5452354542
js_free_shape_null(ctx->rt, sh);
5452454543
cr->prop_offset[i] = prop_offset;
5452554544
end:
54526-
return ch->index;
54545+
icu->offset = ch->index;
5452754546
}
5452854547

5452954548
/* CallSite */

0 commit comments

Comments
 (0)