Closed Bug 699446 Opened 13 years ago Closed 13 years ago

Add cache for constructing new objects from the VM

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: bhackett1024, Assigned: bhackett1024)

References

Details

Attachments

(2 files)

Attached patch patch (deleted) — Splinter Review
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
Attachment #571679 - Flags: review?(luke)
Blocks: ObjectShrink
Attached patch followup (deleted) — Splinter Review
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.
Assignee: general → bhackett1024
Attachment #574647 - Flags: review?(luke)
Attachment #574647 - Flags: review?(luke) → review+
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?
Attachment #571679 - Flags: review?(luke) → review+
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: