699446 - Add cache for constructing new objects from the VM

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Brian Hackett [Laid off!]]

Attachment: patch

Constructing objects in the VM is more expensive with the JM smaller object format, as certain things which used to be easily accessible but wasteful on memory (JSObject::newType, sharedNonNativeMap, ...) have their information encoded via hashtables with a relatively expensive lookup.

The attached patch adds a cache to the VM to make repetitive creation of similar objects much faster.  The cache is modeled on the property cache --- a fixed number of entries where fills will clobber existing entries, and wiped on GC.

On this test (modeled after a Dromaeo test):

function foo() {
  var ret = [], i = 1024;
  function test() {
    for ( var j = 0; j < i * 10; j++ )
      ret = new Array(i);
  }
  for (var i = 0; i < 1000; i++)
    test();
}
foo();

js -m -n (trunk):  617
js -m -n (JM old): 924
js -m -n (JM new): 562

Comment 1

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/projects/jaegermonkey/rev/daa488a2e663

Comment 2

[Brian Hackett [Laid off!]]

Attachment: followup

Followup fix to inhibit this cache when constructing objects whose class does not have a cached proto key.  Creating these objects requires property lookups to get global[className].prototype, where dynamic changes on either property would render the cached entry incorrect.

Comment 3

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/projects/jaegermonkey/rev/f6b97927b0ea

Comment 4

[Luke Wagner [:luke]]

Comment on attachment 571679 [details] [diff] [review]
patch

Review of attachment 571679 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsgcinlines.h
@@ +400,5 @@
>  
> +/* Alternate form which allocates a GC thing if doing so cannot trigger a GC. */
> +template <typename T>
> +inline T *
> +TryNewGCThing(JSContext *cx, js::gc::AllocKind kind, size_t thingSize)

Could you move this (and NewGCThing above it) into namespace js?

::: js/src/jsobj.h
@@ +1509,5 @@
> + * Cache for speeding up repetitive creation of objects in the VM.
> + * When an object is created which matches the criteria in the 'key' section
> + * below, an entry is filled with the resulting object.
> + */
> +struct NewObjectCache

Could you make this be a class with private data members?

@@ +1527,5 @@
> +         * - Prototype for the object (cannot be global). The object's parent
> +         *   will be the prototype's parent.
> +         *
> +         * - Type for the object. The object's parent will be the type's
> +         *   prototype's parent.

This is a good comment.  Could you make a fill() overload for each case (they could all call a private fill() that actually did the work)?  That would give static checking between cases 1,2 and case 3 and then you could have a dynamic check for global vs. non-global.

@@ +1550,5 @@
> +    Entry entries[41];
> +
> +    void reset() { PodZero(this); }
> +
> +    bool lookup(Class *clasp, gc::Cell *key, gc::AllocKind kind, Entry **pentry)

So it's easier to read, could you define this next to NewObjectCache::Entry::fill?

@@ +1553,5 @@
> +
> +    bool lookup(Class *clasp, gc::Cell *key, gc::AllocKind kind, Entry **pentry)
> +    {
> +        jsuword hash = (jsuword(clasp) ^ jsuword(key)) + kind;
> +        Entry *entry = *pentry = &entries[hash % JS_ARRAY_LENGTH(entries)];

entries's length is not a power of 2 so this will actually do the div, not mask.  Any reason not to make give entries 32 or 64 elements?