From df423c5ebb263ed7340e5d84f8503090218c0fa6 Mon Sep 17 00:00:00 2001 From: Sami Vaarala Date: Mon, 19 Dec 2016 10:19:34 +0200 Subject: [PATCH 1/6] Property caching design documentation --- doc/property-cache.rst | 188 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 188 insertions(+) create mode 100644 doc/property-cache.rst diff --git a/doc/property-cache.rst b/doc/property-cache.rst new file mode 100644 index 0000000000..0f41b0b023 --- /dev/null +++ b/doc/property-cache.rst @@ -0,0 +1,188 @@ +============== +Property cache +============== + +This document describes property caching, a mechanism added in Duktape 2.1.0 +to speed up property reads (and maybe writes in the future). + +Data structure +============== + +The property cache is an array of entries to speed up GETPROP operations. +Each entry contains: + +* Object and key reference for original lookup. The object reference stored + is for the lookup starting point, which is often different than the object + actually holding the property (inheritance). + +* Property value. + +* Generation count to allow quick invalidation. + +All key/value references are borrowed. Only concrete properties (not getters +or virtual property values) can be cached due to the property value not being +reference counted. In other words, when a property is cached: + +* There must be a guarantee that the object, key, and value are reachable by + some means. + +* A GETPROP operation from the same starting point using the same key would + yield the same value. + +The ``duk_heap`` structure holds the property cache which is just an array of +entries of (preferably) 2^N size. The heap structure also holds the current +generation count which starts from 1; entries with generation count 0 are thus +ignored automatically. + +Read handling +============= + +* When GETPROP is called, compute a property cache lookup key by combining + object pointer (for lack of an object hash) with the key string hash. + +* Check the entry at the hash index. If the generation count doesn't match + current generation count (which is a heap-wide value), the entry must be + ignored as obsolete. Otherwise check if the object and the key match, and + if so, return the cached value. + +* If there's a cache miss, proceed with normal property lookup. There's no + probing sequence. + +* If no property is found, no property cache changes are made. It would be + possible to implement negative property caching. + + - Negative property caching would improve e.g. JSON stringify() performance + because negative .toJSON() lookups could be cached. + +* If a property is found, and there are no conditions to prevent caching, + overwrite the cache entry at prophash: set generation count to current, + set object and key reference (object reference must be for the lookup + starting point -- not the final object), and property value. + +* There is currently no hash probing/chaining: the existing entry is simply + overwritten with the hope that collision are relatively rare in practice. + +* Conditions preventing caching: + + - Value came from a getter: side effects, value may change per lookup. + + - Value came from a Proxy: side effects, value may change per lookup. + + - Value is virtual and may change per lookup. Virtual values that won't + change over time can be read cached however. + + - Array index lookups probably shouldn't be cached because the same index + is not usually read many times for caching to be useful. These entries + would also purge useful entries unnecessarily. + +Invalidation +============ + +Invalidation must be triggered by any operation that might compromise the +cache integrity requirements, i.e.: + +* The operation might cause object, key, or value (all borrowed) to become + unreachable, thus leading to memory unsafe behavior. + +* The operation might change the property value if a GETPROP were done. + For example, looking up a property storage location is by itself not an + issue, but overwriting the value there is. + +The basic challenge with partial invalidation where only actually affected +entries are removed is that: (1) knowing what entries to remove requires +complicated tracking state, and (2) actually scrubbing affected entries +may require multiple lookups if a change in an inherited property affects +multiple lookup cache entries. + +The current solution is thus to invalidate the entire cache very cheaply by +using a generation count. Entries whose generation count don't match current +are entirely ignored, so that simply bumping the current generation count is +enough to invalidate all entries without touching the entries themselves. + +Technically, if the generation count wraps, entries should be scrubbed +fully. This can be achieved by detecting that the generation count became +zero, setting the whole cache to zero, and then bumping the generation count +once more to ignore the zeroed entries. + +Operations that require invalidation include: + +* PUTPROP + +* DELPROP + +* defineProperty() and all its variants + +* Changes to object internal prototypes: duk_set_prototype(), + Object.setPrototypeOf(), etc. Internal direct prototype changes when there's + any possibility a property access might have happened. + +* Any code that looks up an existing property and modifies its storage location + directly. Easiest approach, at least initially, is to invalide the cache on + that storage lookup. + +* Any code modifying structures that affect non-array-index virtual properties. + For example, writing to ``duk_harray.length`` which appears as a virtual + ``.length`` property. (Needed if virtual ``.length`` is cached; generally + virtual properties cannot be cached because heap-allocated borrowed values + can't be used for dynamic values.) + +* Mark-and-sweep might do compaction which affects property storage locations. + But currently only the value, not its storage location, is cached so that + compaction by itself is not an issue. + +* Mark-and-sweep may finalize objects and then free them however. + +* Assuming that the property keys are reachable through the object, there + should be no chance that the cached key would be freed without the object + being freed. + +* An object may be freed however without any property related operation + taking place. There are only a few places where the object is actually + freed, and those locations can invalidate the property cache. + +Future work +=========== + +* Caching "own property" lookups would speed up some cases where the same + inheritance path is used by multiple lookups. Intuitively this would seem + to be of less practical use. + +* Caching property misses would be straightforward; use DUK_TVAL_SET_UNUSED() + to flag the value as missing. However, negative caching seems of little + practical use because most repeated property lookups are for property hits. + +* Caching property writes would be possible by caching the storage location + rather than the value only. Property attributes can be cached to allow + writes to be validated. Alternatively, if only writable properties are + cached, writability can be assumed if the property is found in the cache. + +* Caching array reads/writes would be possible to some extent, but would + require a slightly different approach to avoid polluting the property + cache with array indices. The caching approach could speed up locating + the ultimate array/array-like object for the index read/write to avoid + walking through the inheritance chain. However, subclassed Arrays or + typed arrays are relatively rare (though Node.js Buffer instances inherit + from Uint8Array). + +* Caching variable reads using the initial property read caching (value only) + approach doesn't work because every executor register write would need to + invalidate the read cache. However, if the cache contained a storage + reference, variable location caching would be possible and could be applied + to slow path read/write references. Inherited properties (scope chain) + could also be cached, provided that there are no Proxy object bindings. + Value stack resize and call handling would need to invalidate the variable + lookup cache because the storage locations might change. + +* Hash probing for the cache locations might help some cases where property + lookup hashes collide. However, it adds another step to the property + lookup, and more importantly causes another cache line to be fetched. + So it may be of relatively little practical value, at least when the + property cache can just be made larger instead. + +* Making ``duk_propcache_entry`` size 2^N would speed up the cache lookup + and align the lookups better. + +* Caching own properties instead of GETPROP/PUTPROP would reduce the benefit + of caching for inheritance chains, but would make caching simple otherwise. + For example, a property creation for one object wouldn't invalidate cached + properties of another because inheritance chains don't matter for caching. From 079394470e8ea727d3b7881265d4d428eefa5407 Mon Sep 17 00:00:00 2001 From: Sami Vaarala Date: Mon, 19 Dec 2016 10:47:01 +0200 Subject: [PATCH 2/6] Implement initial version of property caching --- src-input/duk_bi_array.c | 10 ++++++ src-input/duk_forwdecl.h | 6 ++-- src-input/duk_heap.h | 28 ++++++++++++++-- src-input/duk_heap_alloc.c | 7 +++- src-input/duk_heap_markandsweep.c | 2 ++ src-input/duk_heap_propcache.c | 55 +++++++++++++++++++++++++++++++ src-input/duk_heap_stringcache.c | 12 +++---- src-input/duk_hobject.h | 5 ++- src-input/duk_hobject_misc.c | 2 ++ src-input/duk_hobject_props.c | 38 ++++++++++++++++++++- 10 files changed, 152 insertions(+), 13 deletions(-) create mode 100644 src-input/duk_heap_propcache.c diff --git a/src-input/duk_bi_array.c b/src-input/duk_bi_array.c index 334f1c7e56..22df207134 100644 --- a/src-input/duk_bi_array.c +++ b/src-input/duk_bi_array.c @@ -414,6 +414,11 @@ DUK_LOCAL duk_ret_t duk__array_pop_fastpath(duk_hthread *thr, duk_harray *h_arr) return 0; } + /* Index properties are not cached but .length is, so invalidate + * property cache. + */ + duk_propcache_invalidate(thr); + len--; h_arr->length = len; @@ -500,6 +505,11 @@ DUK_LOCAL duk_ret_t duk__array_push_fastpath(duk_hthread *thr, duk_harray *h_arr return 0; } + /* Index properties are not cached but .length is, so invalidate + * property cache. + */ + duk_propcache_invalidate(thr); + tv_src = thr->valstack_bottom; tv_dst = tv_arraypart + len; for (i = 0; i < n; i++) { diff --git a/src-input/duk_forwdecl.h b/src-input/duk_forwdecl.h index b001b3c5b8..d7b8de289a 100644 --- a/src-input/duk_forwdecl.h +++ b/src-input/duk_forwdecl.h @@ -44,9 +44,10 @@ struct duk_breakpoint; struct duk_activation; struct duk_catcher; -struct duk_strcache; +struct duk_strcache_entry; struct duk_ljstate; struct duk_strtab_entry; +struct duk_propcache_entry; #if defined(DUK_USE_DEBUG) struct duk_fixedbuffer; @@ -104,9 +105,10 @@ typedef struct duk_breakpoint duk_breakpoint; typedef struct duk_activation duk_activation; typedef struct duk_catcher duk_catcher; -typedef struct duk_strcache duk_strcache; +typedef struct duk_strcache_entry duk_strcache_entry; typedef struct duk_ljstate duk_ljstate; typedef struct duk_strtab_entry duk_strtab_entry; +typedef struct duk_propcache_entry duk_propcache_entry; #if defined(DUK_USE_DEBUG) typedef struct duk_fixedbuffer duk_fixedbuffer; diff --git a/src-input/duk_heap.h b/src-input/duk_heap.h index b4e9a8e79c..c381dfa42d 100644 --- a/src-input/duk_heap.h +++ b/src-input/duk_heap.h @@ -148,6 +148,9 @@ #define DUK_HEAP_STRCACHE_SIZE 4 #define DUK_HEAP_STRINGCACHE_NOCACHE_LIMIT 16 /* strings up to the this length are not cached */ +/* Propache is used for speeding up property accesses. */ +#define DUK_HEAP_PROPCACHE_SIZE 4096 + /* Some list management macros. */ #define DUK_HEAP_INSERT_INTO_HEAP_ALLOCATED(heap,hdr) duk_heap_insert_into_heap_allocated((heap), (hdr)) #if defined(DUK_USE_REFERENCE_COUNTING) @@ -291,12 +294,24 @@ struct duk_breakpoint { * Thus, string caches are now at the heap level now. */ -struct duk_strcache { +struct duk_strcache_entry { duk_hstring *h; duk_uint32_t bidx; duk_uint32_t cidx; }; +/* + * Property cache + */ + +struct duk_propcache_entry { + /* All references are borrowed. */ + duk_hobject *obj_lookup; + duk_hstring *key_lookup; + duk_tval value; + duk_uint32_t generation; +}; + /* * Longjmp state, contains the information needed to perform a longjmp. * Longjmp related values are written to value1, value2, and iserror. @@ -545,7 +560,11 @@ struct duk_heap { /* String access cache (codepoint offset -> byte offset) for fast string * character looping; 'weak' reference which needs special handling in GC. */ - duk_strcache strcache[DUK_HEAP_STRCACHE_SIZE]; + duk_strcache_entry strcache[DUK_HEAP_STRCACHE_SIZE]; + + /* Property access cache. */ + duk_propcache_entry propcache[DUK_HEAP_PROPCACHE_SIZE]; + duk_uint32_t propcache_generation; /* Built-in strings. */ #if defined(DUK_USE_ROM_STRINGS) @@ -674,4 +693,9 @@ DUK_INTERNAL_DECL void duk_heap_mark_and_sweep(duk_heap *heap, duk_small_uint_t DUK_INTERNAL_DECL duk_uint32_t duk_heap_hashstring(duk_heap *heap, const duk_uint8_t *str, duk_size_t len); +DUK_INTERNAL_DECL void duk_propcache_invalidate_heap(duk_heap *heap); +DUK_INTERNAL_DECL void duk_propcache_invalidate(duk_hthread *thr); +DUK_INTERNAL_DECL duk_tval *duk_propcache_lookup(duk_hthread *thr, duk_hobject *obj, duk_hstring *key); +DUK_INTERNAL_DECL void duk_propcache_insert(duk_hthread *thr, duk_hobject *obj, duk_hstring *key, duk_tval *storage); + #endif /* DUK_HEAP_H_INCLUDED */ diff --git a/src-input/duk_heap_alloc.c b/src-input/duk_heap_alloc.c index 8e3825b1b7..c2c70f84c6 100644 --- a/src-input/duk_heap_alloc.c +++ b/src-input/duk_heap_alloc.c @@ -22,6 +22,9 @@ DUK_INTERNAL void duk_free_hobject(duk_heap *heap, duk_hobject *h) { DUK_ASSERT(heap != NULL); DUK_ASSERT(h != NULL); + /* FIXME: heap ref only */ + duk_propcache_invalidate_heap(heap); + DUK_FREE(heap, DUK_HOBJECT_GET_PROPS(heap, h)); if (DUK_HOBJECT_IS_COMPFUNC(h)) { @@ -680,7 +683,8 @@ DUK_LOCAL void duk__dump_type_sizes(void) { DUK__DUMPSZ(duk_heap); DUK__DUMPSZ(duk_activation); DUK__DUMPSZ(duk_catcher); - DUK__DUMPSZ(duk_strcache); + DUK__DUMPSZ(duk_strcache_entry); + DUK__DUMPSZ(duk_propcache_entry); DUK__DUMPSZ(duk_ljstate); DUK__DUMPSZ(duk_fixedbuffer); DUK__DUMPSZ(duk_bitdecoder_ctx); @@ -935,6 +939,7 @@ duk_heap *duk_heap_alloc(duk_alloc_function alloc_func, } #endif #endif /* DUK_USE_ROM_STRINGS */ + res->propcache_generation = 1; /* invalidate entries with 0 generation */ #if defined(DUK_USE_DEBUGGER_SUPPORT) res->dbg_read_cb = NULL; res->dbg_write_cb = NULL; diff --git a/src-input/duk_heap_markandsweep.c b/src-input/duk_heap_markandsweep.c index 408283d0b8..a41f8bd24f 100644 --- a/src-input/duk_heap_markandsweep.c +++ b/src-input/duk_heap_markandsweep.c @@ -1139,6 +1139,8 @@ DUK_INTERNAL void duk_heap_mark_and_sweep(duk_heap *heap, duk_small_uint_t flags DUK_ASSERT(heap->heap_thread != NULL); DUK_ASSERT(heap->heap_thread->valstack != NULL); + /* FIXME: property cache invalidation or comment why not needed. */ + DUK_D(DUK_DPRINT("garbage collect (mark-and-sweep) starting, requested flags: 0x%08lx, effective flags: 0x%08lx", (unsigned long) flags, (unsigned long) (flags | heap->ms_base_flags))); diff --git a/src-input/duk_heap_propcache.c b/src-input/duk_heap_propcache.c new file mode 100644 index 0000000000..cc1b5bbf65 --- /dev/null +++ b/src-input/duk_heap_propcache.c @@ -0,0 +1,55 @@ +#include "duk_internal.h" + +DUK_LOCAL DUK_NOINLINE void duk__propcache_scrub(duk_heap *heap) { + /* Explicit scrub to avoid reuse. Generation counts are set + * to zero, and the generation count is then bumped once more + * to one to invalidate the scrubbed entries. + */ + DUK_MEMZERO((void *) heap->propcache, sizeof(heap->propcache)); + heap->propcache_generation++; +} + +DUK_INTERNAL DUK_ALWAYS_INLINE void duk_propcache_invalidate_heap(duk_heap *heap) { + if (DUK_UNLIKELY(++heap->propcache_generation == 0)) { + duk__propcache_scrub(heap); + } +} + +DUK_INTERNAL DUK_ALWAYS_INLINE void duk_propcache_invalidate(duk_hthread *thr) { + duk_propcache_invalidate_heap(thr->heap); +} + +DUK_LOCAL duk_size_t duk__compute_prophash(duk_hobject *obj, duk_hstring *key) { + duk_size_t prophash; + prophash = ((duk_size_t) obj) ^ ((duk_size_t) DUK_HSTRING_GET_HASH(key)); + prophash = prophash % DUK_HEAP_PROPCACHE_SIZE; /* FIXME: force to be a mask */ + return prophash; +} + +DUK_INTERNAL duk_tval *duk_propcache_lookup(duk_hthread *thr, duk_hobject *obj, duk_hstring *key) { + duk_size_t prophash; + duk_propcache_entry *ent; + + prophash = duk__compute_prophash(obj, key); + ent = thr->heap->propcache + prophash; + + if (ent->generation == thr->heap->propcache_generation && + ent->obj_lookup == obj && + ent->key_lookup == key) { + return &ent->value; + } + + return NULL; +} + +DUK_INTERNAL void duk_propcache_insert(duk_hthread *thr, duk_hobject *obj, duk_hstring *key, duk_tval *tv) { + duk_size_t prophash; + duk_propcache_entry *ent; + + prophash = duk__compute_prophash(obj, key); + ent = thr->heap->propcache + prophash; + ent->generation = thr->heap->propcache_generation; + ent->obj_lookup = obj; + ent->key_lookup = key; + DUK_TVAL_SET_TVAL(&ent->value, tv); +} diff --git a/src-input/duk_heap_stringcache.c b/src-input/duk_heap_stringcache.c index b092773f01..4eee77e39e 100644 --- a/src-input/duk_heap_stringcache.c +++ b/src-input/duk_heap_stringcache.c @@ -20,7 +20,7 @@ DUK_INTERNAL void duk_heap_strcache_string_remove(duk_heap *heap, duk_hstring *h) { duk_small_int_t i; for (i = 0; i < DUK_HEAP_STRCACHE_SIZE; i++) { - duk_strcache *c = heap->strcache + i; + duk_strcache_entry *c = heap->strcache + i; if (c->h == h) { DUK_DD(DUK_DDPRINT("deleting weak strcache reference to hstring %p from heap %p", (void *) h, (void *) heap)); @@ -92,7 +92,7 @@ DUK_LOCAL const duk_uint8_t *duk__scan_backwards(const duk_uint8_t *p, const duk DUK_INTERNAL duk_uint_fast32_t duk_heap_strcache_offset_char2byte(duk_hthread *thr, duk_hstring *h, duk_uint_fast32_t char_offset) { duk_heap *heap; - duk_strcache *sce; + duk_strcache_entry *sce; duk_uint_fast32_t byte_offset; duk_small_int_t i; duk_bool_t use_cache; @@ -145,14 +145,14 @@ DUK_INTERNAL duk_uint_fast32_t duk_heap_strcache_offset_char2byte(duk_hthread *t #if defined(DUK_USE_DEBUG_LEVEL) && (DUK_USE_DEBUG_LEVEL >= 2) DUK_DDD(DUK_DDDPRINT("stringcache before char2byte (using cache):")); for (i = 0; i < DUK_HEAP_STRCACHE_SIZE; i++) { - duk_strcache *c = heap->strcache + i; + duk_strcache_entry *c = heap->strcache + i; DUK_DDD(DUK_DDDPRINT(" [%ld] -> h=%p, cidx=%ld, bidx=%ld", (long) i, (void *) c->h, (long) c->cidx, (long) c->bidx)); } #endif for (i = 0; i < DUK_HEAP_STRCACHE_SIZE; i++) { - duk_strcache *c = heap->strcache + i; + duk_strcache_entry *c = heap->strcache + i; if (c->h == h) { sce = c; @@ -281,7 +281,7 @@ DUK_INTERNAL duk_uint_fast32_t duk_heap_strcache_offset_char2byte(duk_hthread *t * C <- sce ==> B * D D */ - duk_strcache tmp; + duk_strcache_entry tmp; tmp = *sce; DUK_MEMMOVE((void *) (&heap->strcache[1]), @@ -294,7 +294,7 @@ DUK_INTERNAL duk_uint_fast32_t duk_heap_strcache_offset_char2byte(duk_hthread *t #if defined(DUK_USE_DEBUG_LEVEL) && (DUK_USE_DEBUG_LEVEL >= 2) DUK_DDD(DUK_DDDPRINT("stringcache after char2byte (using cache):")); for (i = 0; i < DUK_HEAP_STRCACHE_SIZE; i++) { - duk_strcache *c = heap->strcache + i; + duk_strcache_entry *c = heap->strcache + i; DUK_DDD(DUK_DDDPRINT(" [%ld] -> h=%p, cidx=%ld, bidx=%ld", (long) i, (void *) c->h, (long) c->cidx, (long) c->bidx)); } diff --git a/src-input/duk_hobject.h b/src-input/duk_hobject.h index e819e51556..0cf740f498 100644 --- a/src-input/duk_hobject.h +++ b/src-input/duk_hobject.h @@ -655,6 +655,7 @@ #define DUK_HOBJECT_GET_PROTOTYPE(heap,h) \ ((duk_hobject *) DUK_USE_HEAPPTR_DEC16((heap)->heap_udata, (h)->prototype16)) #define DUK_HOBJECT_SET_PROTOTYPE(heap,h,x) do { \ + /* FIXME: caller property invalidation */ \ (h)->prototype16 = DUK_USE_HEAPPTR_ENC16((heap)->heap_udata, (void *) (x)); \ } while (0) #else @@ -665,7 +666,9 @@ } while (0) #endif -/* Set prototype, DECREF earlier value, INCREF new value (tolerating NULLs). */ +/* Set prototype, DECREF earlier value, INCREF new value (tolerating NULLs). + * Also automatically invalidates property cache. + */ #define DUK_HOBJECT_SET_PROTOTYPE_UPDREF(thr,h,p) duk_hobject_set_prototype_updref((thr), (h), (p)) /* Set initial prototype, assume NULL previous prototype, INCREF new value, diff --git a/src-input/duk_hobject_misc.c b/src-input/duk_hobject_misc.c index 058bf18ce7..8030da9561 100644 --- a/src-input/duk_hobject_misc.c +++ b/src-input/duk_hobject_misc.c @@ -43,10 +43,12 @@ DUK_INTERNAL void duk_hobject_set_prototype_updref(duk_hthread *thr, duk_hobject tmp = DUK_HOBJECT_GET_PROTOTYPE(thr->heap, h); DUK_HOBJECT_SET_PROTOTYPE(thr->heap, h, p); DUK_HOBJECT_INCREF_ALLOWNULL(thr, p); /* avoid problems if p == h->prototype */ + duk_propcache_invalidate(thr); /* must be done just before finalizers */ DUK_HOBJECT_DECREF_ALLOWNULL(thr, tmp); #else DUK_ASSERT(h); DUK_UNREF(thr); DUK_HOBJECT_SET_PROTOTYPE(thr->heap, h, p); + duk_propcache_invalidate(thr); #endif } diff --git a/src-input/duk_hobject_props.c b/src-input/duk_hobject_props.c index eda1fb4a00..a476c25a8f 100644 --- a/src-input/duk_hobject_props.c +++ b/src-input/duk_hobject_props.c @@ -2334,6 +2334,7 @@ DUK_LOCAL duk_bool_t duk__putprop_fastpath_bufobj_tval(duk_hthread *thr, duk_hob DUK_INTERNAL duk_bool_t duk_hobject_getprop(duk_hthread *thr, duk_tval *tv_obj, duk_tval *tv_key) { duk_tval tv_obj_copy; duk_tval tv_key_copy; + duk_hobject *orig_base; duk_hobject *curr = NULL; duk_hstring *key = NULL; duk_uint32_t arr_idx = DUK__NO_ARRAY_INDEX; @@ -2684,6 +2685,22 @@ DUK_INTERNAL duk_bool_t duk_hobject_getprop(duk_hthread *thr, duk_tval *tv_obj, DUK_ASSERT(curr != NULL); DUK_ASSERT(key != NULL); + orig_base = curr; + { + /* FIXME: when base is primitive, we cache the "wrong" property, + * which is fine. + */ + duk_tval *storage; + storage = duk_propcache_lookup(thr, curr, key); + if (storage) { + DUK_D(DUK_DPRINT("cached lookup %!O -> %!T", key, storage)); + duk_pop(ctx); + duk_push_tval(ctx, storage); + /* FIXME: assume no post process? */ + return 1; + } + } + sanity = DUK_HOBJECT_PROTOTYPE_CHAIN_SANITY; do { if (!duk__get_own_propdesc_raw(thr, curr, key, arr_idx, &desc, DUK_GETDESC_FLAG_PUSH_VALUE)) { @@ -2694,6 +2711,9 @@ DUK_INTERNAL duk_bool_t duk_hobject_getprop(duk_hthread *thr, duk_tval *tv_obj, /* accessor with defined getter */ DUK_ASSERT((desc.flags & DUK_PROPDESC_FLAG_ACCESSOR) != 0); + DUK_D(DUK_DPRINT("prevent caching, getter")); + orig_base = NULL; + duk_pop_unsafe(thr); /* [key undefined] -> [key] */ duk_push_hobject(thr, desc.get); duk_push_tval(thr, tv_obj); /* note: original, uncoerced base */ @@ -2737,6 +2757,7 @@ DUK_INTERNAL duk_bool_t duk_hobject_getprop(duk_hthread *thr, duk_tval *tv_obj, * Not found */ + /* XXX: No negative property caching at present. */ duk_to_undefined(thr, -1); /* [key] -> [undefined] (default value) */ DUK_DDD(DUK_DDDPRINT("-> %!T (not found)", (duk_tval *) duk_get_tval(thr, -1))); @@ -2804,7 +2825,12 @@ DUK_INTERNAL duk_bool_t duk_hobject_getprop(duk_hthread *thr, duk_tval *tv_obj, } #endif /* !DUK_USE_NONSTD_FUNC_CALLER_PROPERTY */ - duk_remove_m2(thr); /* [key result] -> [result] */ + if (orig_base && arr_idx == DUK__NO_ARRAY_INDEX) { /* FIXME: condition */ + /* FIXME: other conditions, e.g. not a getter */ + duk_propcache_insert(thr, orig_base, key, DUK_GET_TVAL_NEGIDX(ctx, -1)); + } + + duk_remove_m2(ctx); /* [key result] -> [result] */ DUK_DDD(DUK_DDDPRINT("-> %!T (found)", (duk_tval *) duk_get_tval(thr, -1))); return 1; @@ -3352,6 +3378,8 @@ DUK_INTERNAL duk_bool_t duk_hobject_putprop(duk_hthread *thr, duk_tval *tv_obj, DUK_STATS_INC(thr->heap, stats_putprop_all); + duk_propcache_invalidate(thr); + /* * Make a copy of tv_obj, tv_key, and tv_val to avoid any issues of * them being invalidated by a valstack resize. @@ -4247,6 +4275,8 @@ DUK_INTERNAL duk_bool_t duk_hobject_delprop_raw(duk_hthread *thr, duk_hobject *o duk_bool_t throw_flag; duk_bool_t force_flag; + duk_propcache_invalidate(thr); + throw_flag = (flags & DUK_DELPROP_FLAG_THROW); force_flag = (flags & DUK_DELPROP_FLAG_FORCE); @@ -4606,6 +4636,8 @@ DUK_INTERNAL void duk_hobject_define_property_internal(duk_hthread *thr, duk_hob duk_tval *tv2 = NULL; duk_small_uint_t propflags = flags & DUK_PROPDESC_FLAGS_MASK; /* mask out flags not actually stored */ + duk_propcache_invalidate(thr); + DUK_DDD(DUK_DDDPRINT("define new property (internal): thr=%p, obj=%!O, key=%!O, flags=0x%02lx, val=%!T", (void *) thr, (duk_heaphdr *) obj, (duk_heaphdr *) key, (unsigned long) flags, (duk_tval *) duk_get_tval(thr, -1))); @@ -4726,6 +4758,8 @@ DUK_INTERNAL void duk_hobject_define_property_internal_arridx(duk_hthread *thr, duk_hstring *key; duk_tval *tv1, *tv2; + duk_propcache_invalidate(thr); + DUK_DDD(DUK_DDDPRINT("define new property (internal) arr_idx fast path: thr=%p, obj=%!O, " "arr_idx=%ld, flags=0x%02lx, val=%!T", (void *) thr, obj, (long) arr_idx, (unsigned long) flags, @@ -5093,6 +5127,8 @@ duk_bool_t duk_hobject_define_property_helper(duk_hthread *thr, DUK_ASSERT_VALSTACK_SPACE(thr, DUK__VALSTACK_SPACE); + duk_propcache_invalidate(thr); + /* All the flags fit in 16 bits, so will fit into duk_bool_t. */ has_writable = (defprop_flags & DUK_DEFPROP_HAVE_WRITABLE); From 2ea67b71791cd6c1d33e3fd13caafaf667d960f6 Mon Sep 17 00:00:00 2001 From: Sami Vaarala Date: Mon, 19 Dec 2016 10:47:10 +0200 Subject: [PATCH 3/6] Add propcache files to dist/configure --- tools/configure.py | 1 + util/dist.py | 1 + 2 files changed, 2 insertions(+) diff --git a/tools/configure.py b/tools/configure.py index 3be232f15d..914c86d6ee 100644 --- a/tools/configure.py +++ b/tools/configure.py @@ -462,6 +462,7 @@ def default_from_script_path(optname, orig, alternatives): 'duk_heap_markandsweep.c', 'duk_heap_memory.c', 'duk_heap_misc.c', + 'duk_heap_propcache.c', 'duk_heap_refcount.c', 'duk_heap_stringcache.c', 'duk_heap_stringtable.c', diff --git a/util/dist.py b/util/dist.py index 7e9e4981da..bc527b5b3b 100644 --- a/util/dist.py +++ b/util/dist.py @@ -370,6 +370,7 @@ def main(): 'duk_heap_markandsweep.c', 'duk_heap_memory.c', 'duk_heap_misc.c', + 'duk_heap_propcache.c', 'duk_heap_refcount.c', 'duk_heap_stringcache.c', 'duk_heap_stringtable.c', From 7b970c860af138219f2cff52dc767ad54dc10452 Mon Sep 17 00:00:00 2001 From: Sami Vaarala Date: Mon, 19 Dec 2016 11:12:36 +0200 Subject: [PATCH 4/6] Add a few microbenchmarks for property caching --- tests/perf/test-prop-read-long-inherit.js | 20 +++++++++++++++++++ tests/perf/test-prop-read-math-pi.js | 14 +++++++++++++ tests/perf/test-prop-write-long-inherit.js | 23 ++++++++++++++++++++++ 3 files changed, 57 insertions(+) create mode 100644 tests/perf/test-prop-read-long-inherit.js create mode 100644 tests/perf/test-prop-read-math-pi.js create mode 100644 tests/perf/test-prop-write-long-inherit.js diff --git a/tests/perf/test-prop-read-long-inherit.js b/tests/perf/test-prop-read-long-inherit.js new file mode 100644 index 0000000000..e8f93f1963 --- /dev/null +++ b/tests/perf/test-prop-read-long-inherit.js @@ -0,0 +1,20 @@ +function test() { + var i; + var obj; + + obj = { foo: 123 }; + for (i = 0; i < 100; i++) { + obj = Object.create(obj); +  } + + for (i = 0; i < 1e7; i++) { + void obj.foo; + } +} + +try { + test(); +} catch (e) { + print(e.stack || e); + throw e; +} diff --git a/tests/perf/test-prop-read-math-pi.js b/tests/perf/test-prop-read-math-pi.js new file mode 100644 index 0000000000..7cb2111b73 --- /dev/null +++ b/tests/perf/test-prop-read-math-pi.js @@ -0,0 +1,14 @@ +function test() { + var i; + + for (i = 0; i < 1e7; i++) { + void Math.PI + } +} + +try { + test(); +} catch (e) { + print(e.stack || e); + throw e; +} diff --git a/tests/perf/test-prop-write-long-inherit.js b/tests/perf/test-prop-write-long-inherit.js new file mode 100644 index 0000000000..174ec7f3b0 --- /dev/null +++ b/tests/perf/test-prop-write-long-inherit.js @@ -0,0 +1,23 @@ +function test() { + var i; + var obj; + + obj = { foo: 123 }; + for (i = 0; i < 100; i++) { + obj = Object.create(obj); +  } + + for (i = 0; i < 1e7; i++) { + // Because a write always causes an own property to be created, + // there's no difference between a shallow and deep object except + // on the very first write. + obj.foo = 123; + } +} + +try { + test(); +} catch (e) { + print(e.stack || e); + throw e; +} From ffb8a6adcbd619585adad4c6f1608d04288caae7 Mon Sep 17 00:00:00 2001 From: Sami Vaarala Date: Sun, 8 Jan 2017 02:58:09 +0200 Subject: [PATCH 5/6] Prototype with propcache storage caching --- src-input/duk_heap.h | 2 +- src-input/duk_heap_propcache.c | 10 +++++-- src-input/duk_hobject_props.c | 53 +++++++++++++++++++++++++++++++--- 3 files changed, 58 insertions(+), 7 deletions(-) diff --git a/src-input/duk_heap.h b/src-input/duk_heap.h index c381dfa42d..fb3a25fd98 100644 --- a/src-input/duk_heap.h +++ b/src-input/duk_heap.h @@ -308,7 +308,7 @@ struct duk_propcache_entry { /* All references are borrowed. */ duk_hobject *obj_lookup; duk_hstring *key_lookup; - duk_tval value; + duk_tval *val_storage; duk_uint32_t generation; }; diff --git a/src-input/duk_heap_propcache.c b/src-input/duk_heap_propcache.c index cc1b5bbf65..1c24615bce 100644 --- a/src-input/duk_heap_propcache.c +++ b/src-input/duk_heap_propcache.c @@ -5,6 +5,7 @@ DUK_LOCAL DUK_NOINLINE void duk__propcache_scrub(duk_heap *heap) { * to zero, and the generation count is then bumped once more * to one to invalidate the scrubbed entries. */ + DUK_D(DUK_DPRINT("INVALIDATE propcache")); DUK_MEMZERO((void *) heap->propcache, sizeof(heap->propcache)); heap->propcache_generation++; } @@ -33,10 +34,15 @@ DUK_INTERNAL duk_tval *duk_propcache_lookup(duk_hthread *thr, duk_hobject *obj, prophash = duk__compute_prophash(obj, key); ent = thr->heap->propcache + prophash; + DUK_D(DUK_DPRINT("lookup, prophash %lu, gen %lu, ent->gen %lu, ent->obj %p, ent->key %p, ent->val %p", + (unsigned long) prophash, (unsigned long) thr->heap->propcache_generation, + (unsigned long) ent->generation, (void *) ent->obj_lookup, + (void *) ent->key_lookup, (void *) ent->val_storage)); + if (ent->generation == thr->heap->propcache_generation && ent->obj_lookup == obj && ent->key_lookup == key) { - return &ent->value; + return ent->val_storage; /* Storage location. */ } return NULL; @@ -51,5 +57,5 @@ DUK_INTERNAL void duk_propcache_insert(duk_hthread *thr, duk_hobject *obj, duk_h ent->generation = thr->heap->propcache_generation; ent->obj_lookup = obj; ent->key_lookup = key; - DUK_TVAL_SET_TVAL(&ent->value, tv); + ent->val_storage = tv; } diff --git a/src-input/duk_hobject_props.c b/src-input/duk_hobject_props.c index a476c25a8f..6245602be9 100644 --- a/src-input/duk_hobject_props.c +++ b/src-input/duk_hobject_props.c @@ -536,6 +536,9 @@ DUK_INTERNAL void duk_hobject_realloc_props(duk_hthread *thr, DUK_STATS_INC(thr->heap, stats_object_realloc_props); + /* Invalidate property cache. FIXME: also at the end? */ + duk_propcache_invalidate(thr); + /* * Pre resize assertions. */ @@ -2692,10 +2695,11 @@ DUK_INTERNAL duk_bool_t duk_hobject_getprop(duk_hthread *thr, duk_tval *tv_obj, */ duk_tval *storage; storage = duk_propcache_lookup(thr, curr, key); + DUK_D(DUK_DPRINT("propcache GETPROP lookup: %!O %!O -> %p", orig_base, key, (void *) storage)); if (storage) { - DUK_D(DUK_DPRINT("cached lookup %!O -> %!T", key, storage)); - duk_pop(ctx); + DUK_D(DUK_DPRINT("cached GETPROP lookup %!O -> %!T", key, storage)); duk_push_tval(ctx, storage); + duk_remove(ctx, -2); /* FIXME: careful with order */ /* FIXME: assume no post process? */ return 1; } @@ -2825,9 +2829,13 @@ DUK_INTERNAL duk_bool_t duk_hobject_getprop(duk_hthread *thr, duk_tval *tv_obj, } #endif /* !DUK_USE_NONSTD_FUNC_CALLER_PROPERTY */ - if (orig_base && arr_idx == DUK__NO_ARRAY_INDEX) { /* FIXME: condition */ + if (orig_base && arr_idx == DUK__NO_ARRAY_INDEX && desc.e_idx >= 0) { /* FIXME: condition */ /* FIXME: other conditions, e.g. not a getter */ - duk_propcache_insert(thr, orig_base, key, DUK_GET_TVAL_NEGIDX(ctx, -1)); + /* FIXME: note that caching is based on orig_base, but storage location is in 'curr'! */ + duk_tval *tv_storage; + tv_storage = DUK_HOBJECT_E_GET_VALUE_TVAL_PTR(thr->heap, curr, desc.e_idx); + DUK_D(DUK_DPRINT("insert propcache GETPROP %!O", key)); + duk_propcache_insert(thr, orig_base, key, tv_storage); } duk_remove_m2(ctx); /* [key result] -> [result] */ @@ -3353,6 +3361,7 @@ DUK_INTERNAL duk_bool_t duk_hobject_putprop(duk_hthread *thr, duk_tval *tv_obj, duk_tval tv_key_copy; duk_tval tv_val_copy; duk_hobject *orig = NULL; /* NULL if tv_obj is primitive */ + duk_hobject *orig_base; duk_hobject *curr; duk_hstring *key = NULL; duk_propdesc desc; @@ -3378,7 +3387,12 @@ DUK_INTERNAL duk_bool_t duk_hobject_putprop(duk_hthread *thr, duk_tval *tv_obj, DUK_STATS_INC(thr->heap, stats_putprop_all); + /* FIXME... with storage location caching, only need to invalidate + * if new properties are established (may shadow existing chains)? + */ +#if 0 duk_propcache_invalidate(thr); +#endif /* * Make a copy of tv_obj, tv_key, and tv_val to avoid any issues of @@ -3677,6 +3691,23 @@ DUK_INTERNAL duk_bool_t duk_hobject_putprop(duk_hthread *thr, duk_tval *tv_obj, lookup: + DUK_ASSERT(curr != NULL); + orig_base = curr; + + { + /* FIXME: when base is primitive, we cache the "wrong" property, + * which is fine. + */ + duk_tval *storage; + storage = duk_propcache_lookup(thr, orig_base, key); + DUK_D(DUK_DPRINT("propcache PUTPROP lookup: %!O %!O -> %p", orig_base, key, (void *) storage)); + if (storage) { + DUK_D(DUK_DPRINT("cached PUTPROP lookup %!O -> old value %!T", key, storage)); + DUK_TVAL_SET_TVAL_UPDREF(thr, storage, tv_val); + goto success_no_arguments_exotic; + } + } + /* * Check whether the property already exists in the prototype chain. * Note that the actual write goes into the original base object @@ -3901,6 +3932,14 @@ DUK_INTERNAL duk_bool_t duk_hobject_putprop(duk_hthread *thr, duk_tval *tv_obj, if (desc.e_idx >= 0) { tv = DUK_HOBJECT_E_GET_VALUE_TVAL_PTR(thr->heap, orig, desc.e_idx); + + /* FIXME argument exotic condition... */ + if (1) { /* FIXME: condition */ + /* FIXME: other conditions, e.g. not a getter */ + DUK_D(DUK_DPRINT("insert propcache PUTPROP %!O", key)); + duk_propcache_insert(thr, orig_base, key, tv); + } + DUK_DDD(DUK_DDDPRINT("previous entry value: %!iT", (duk_tval *) tv)); DUK_TVAL_SET_TVAL_UPDREF(thr, tv, tv_val); /* side effects; e_idx may be invalidated */ /* don't touch property attributes or hash part */ @@ -4103,6 +4142,12 @@ DUK_INTERNAL duk_bool_t duk_hobject_putprop(duk_hthread *thr, duk_tval *tv_obj, write_to_entry_part: + /* Must invalidate: new property may affect existing inheritance + * chains. + */ + duk_propcache_invalidate(thr); + + /* * Write to entry part */ From 84afa79dd116f0055e26746dbef9cec0c4a4a1ec Mon Sep 17 00:00:00 2001 From: Sami Vaarala Date: Fri, 23 Jun 2017 01:39:40 +0300 Subject: [PATCH 6/6] Some more tweaks, to be squashed --- src-input/duk_heap.h | 4 ++++ src-input/duk_heap_markandsweep.c | 3 +++ src-input/duk_heap_propcache.c | 14 +++++++++----- src-input/duk_hobject_props.c | 18 +++++++++--------- 4 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src-input/duk_heap.h b/src-input/duk_heap.h index fb3a25fd98..20c13d7a24 100644 --- a/src-input/duk_heap.h +++ b/src-input/duk_heap.h @@ -617,6 +617,10 @@ struct duk_heap { duk_int_t stats_putprop_proxy; duk_int_t stats_getvar_all; duk_int_t stats_putvar_all; + duk_int_t stats_propcache_invalidate; + duk_int_t stats_propcache_hit; + duk_int_t stats_propcache_miss; + duk_int_t stats_propcache_insert; #endif }; diff --git a/src-input/duk_heap_markandsweep.c b/src-input/duk_heap_markandsweep.c index a41f8bd24f..584d614ccb 100644 --- a/src-input/duk_heap_markandsweep.c +++ b/src-input/duk_heap_markandsweep.c @@ -1090,6 +1090,9 @@ DUK_LOCAL void duk__dump_stats(duk_heap *heap) { (long) heap->stats_getvar_all)); DUK_D(DUK_DPRINT("stats putvar: all=%ld", (long) heap->stats_putvar_all)); + DUK_D(DUK_DPRINT("stats propcache: invalidate=%ld, hit=%ld, miss=%ld, insert=%ld", + (long) heap->stats_propcache_invalidate, (long) heap->stats_propcache_hit, + (long) heap->stats_propcache_miss, (long) heap->stats_propcache_insert)); } #endif /* DUK_USE_DEBUG */ diff --git a/src-input/duk_heap_propcache.c b/src-input/duk_heap_propcache.c index 1c24615bce..8817f1d918 100644 --- a/src-input/duk_heap_propcache.c +++ b/src-input/duk_heap_propcache.c @@ -5,12 +5,13 @@ DUK_LOCAL DUK_NOINLINE void duk__propcache_scrub(duk_heap *heap) { * to zero, and the generation count is then bumped once more * to one to invalidate the scrubbed entries. */ - DUK_D(DUK_DPRINT("INVALIDATE propcache")); + DUK_DD(DUK_DDPRINT("INVALIDATE propcache")); DUK_MEMZERO((void *) heap->propcache, sizeof(heap->propcache)); heap->propcache_generation++; } DUK_INTERNAL DUK_ALWAYS_INLINE void duk_propcache_invalidate_heap(duk_heap *heap) { + DUK_STATS_INC(heap, stats_propcache_invalidate); if (DUK_UNLIKELY(++heap->propcache_generation == 0)) { duk__propcache_scrub(heap); } @@ -34,17 +35,19 @@ DUK_INTERNAL duk_tval *duk_propcache_lookup(duk_hthread *thr, duk_hobject *obj, prophash = duk__compute_prophash(obj, key); ent = thr->heap->propcache + prophash; - DUK_D(DUK_DPRINT("lookup, prophash %lu, gen %lu, ent->gen %lu, ent->obj %p, ent->key %p, ent->val %p", - (unsigned long) prophash, (unsigned long) thr->heap->propcache_generation, - (unsigned long) ent->generation, (void *) ent->obj_lookup, - (void *) ent->key_lookup, (void *) ent->val_storage)); + DUK_DD(DUK_DDPRINT("lookup, prophash %lu, gen %lu, ent->gen %lu, ent->obj %p, ent->key %p, ent->val %p", + (unsigned long) prophash, (unsigned long) thr->heap->propcache_generation, + (unsigned long) ent->generation, (void *) ent->obj_lookup, + (void *) ent->key_lookup, (void *) ent->val_storage)); if (ent->generation == thr->heap->propcache_generation && ent->obj_lookup == obj && ent->key_lookup == key) { + DUK_STATS_INC(thr->heap, stats_propcache_hit); return ent->val_storage; /* Storage location. */ } + DUK_STATS_INC(thr->heap, stats_propcache_miss); return NULL; } @@ -52,6 +55,7 @@ DUK_INTERNAL void duk_propcache_insert(duk_hthread *thr, duk_hobject *obj, duk_h duk_size_t prophash; duk_propcache_entry *ent; + DUK_STATS_INC(thr->heap, stats_propcache_insert); prophash = duk__compute_prophash(obj, key); ent = thr->heap->propcache + prophash; ent->generation = thr->heap->propcache_generation; diff --git a/src-input/duk_hobject_props.c b/src-input/duk_hobject_props.c index 6245602be9..b6aa36ead9 100644 --- a/src-input/duk_hobject_props.c +++ b/src-input/duk_hobject_props.c @@ -2695,11 +2695,11 @@ DUK_INTERNAL duk_bool_t duk_hobject_getprop(duk_hthread *thr, duk_tval *tv_obj, */ duk_tval *storage; storage = duk_propcache_lookup(thr, curr, key); - DUK_D(DUK_DPRINT("propcache GETPROP lookup: %!O %!O -> %p", orig_base, key, (void *) storage)); + DUK_DD(DUK_DDPRINT("propcache GETPROP lookup: %!O %!O -> %p", orig_base, key, (void *) storage)); if (storage) { - DUK_D(DUK_DPRINT("cached GETPROP lookup %!O -> %!T", key, storage)); - duk_push_tval(ctx, storage); - duk_remove(ctx, -2); /* FIXME: careful with order */ + DUK_DD(DDUK_DPRINT("cached GETPROP lookup %!O -> %!T", key, storage)); + duk_push_tval(thr, storage); + duk_remove(thr, -2); /* FIXME: careful with order */ /* FIXME: assume no post process? */ return 1; } @@ -2834,11 +2834,11 @@ DUK_INTERNAL duk_bool_t duk_hobject_getprop(duk_hthread *thr, duk_tval *tv_obj, /* FIXME: note that caching is based on orig_base, but storage location is in 'curr'! */ duk_tval *tv_storage; tv_storage = DUK_HOBJECT_E_GET_VALUE_TVAL_PTR(thr->heap, curr, desc.e_idx); - DUK_D(DUK_DPRINT("insert propcache GETPROP %!O", key)); + DUK_DD(DUK_DDPRINT("insert propcache GETPROP %!O", key)); duk_propcache_insert(thr, orig_base, key, tv_storage); } - duk_remove_m2(ctx); /* [key result] -> [result] */ + duk_remove_m2(thr); /* [key result] -> [result] */ DUK_DDD(DUK_DDDPRINT("-> %!T (found)", (duk_tval *) duk_get_tval(thr, -1))); return 1; @@ -3700,9 +3700,9 @@ DUK_INTERNAL duk_bool_t duk_hobject_putprop(duk_hthread *thr, duk_tval *tv_obj, */ duk_tval *storage; storage = duk_propcache_lookup(thr, orig_base, key); - DUK_D(DUK_DPRINT("propcache PUTPROP lookup: %!O %!O -> %p", orig_base, key, (void *) storage)); + DUK_DD(DUK_DDPRINT("propcache PUTPROP lookup: %!O %!O -> %p", orig_base, key, (void *) storage)); if (storage) { - DUK_D(DUK_DPRINT("cached PUTPROP lookup %!O -> old value %!T", key, storage)); + DUK_DD(DUK_DDPRINT("cached PUTPROP lookup %!O -> old value %!T", key, storage)); DUK_TVAL_SET_TVAL_UPDREF(thr, storage, tv_val); goto success_no_arguments_exotic; } @@ -3936,7 +3936,7 @@ DUK_INTERNAL duk_bool_t duk_hobject_putprop(duk_hthread *thr, duk_tval *tv_obj, /* FIXME argument exotic condition... */ if (1) { /* FIXME: condition */ /* FIXME: other conditions, e.g. not a getter */ - DUK_D(DUK_DPRINT("insert propcache PUTPROP %!O", key)); + DUK_DD(DUK_DDPRINT("insert propcache PUTPROP %!O", key)); duk_propcache_insert(thr, orig_base, key, tv); }