Implement an initial version of property caching

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.

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++) {

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;

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 *val_storage;
+	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)
@@ -598,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
 };
 
@@ -674,4 +697,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 */

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;

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 */
 
@@ -1139,6 +1142,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)));