1368076 - Optimize GetOwnPropertyKeys

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, enhancement, P3)

Description

[Tom S. (please needinfo tschuster)]

GetOwnPropertyKeys is used by Object.keys and various other builtins internally. I tried adding a cache for this, by using copy-on-write arrays. This didn't really work out, but it's probably salvageable maybe by caching values and creating a new array every time with those.

After that I just worked on optimizing the common case: i.e. no enumerate hooks and no non-dense indexed properties. By avoiding the AutoIdArray and the various other not very optimized parts of GetPropertyKeys, I got a speedup of about 2x.

Comment 1

[Tom S. (please needinfo tschuster)]

Attachment: WIP

After (hopefully) correctly implementing everything we aren't that much fast anymore. Maybe I should look into templatizing the code more, so that we don't have to check flags inside the loop. I also noticed an unlined call to "MutableWrappedPtrOperations<JS::GCVector<jsid, 8ul, js::TempAllocPolicy>, JS::Rooted<JS::GCVector<jsid, 8ul, js::TempAllocPolicy> > >::operator[]".

Comment 2

[Tom S. (please needinfo tschuster)]

Maybe I compiled this with in -O0 or something. Anyway with clang-5.0 this code is only something like 5% faster. Mostly because of the temporary vectors I think.

I am going to do the caching part now instead.

Comment 3

[Tom S. (please needinfo tschuster)]

Attachment: WIP with caching

I tried using copy-on-write arrays for this, but I ran into issues. Maybe the owner array can't be exposed to JS? Anyway this takes a micro benchmark from 340ms to 266ms. I want to avoid copying the cached GCVector for keys though, but I haven't been able to figure out how to make this work. Can we zero-copy move the GCVector into the cache somehow?

Comment 4

[Mayank Bansal]

Lot of work has recently been done in Bug 1836679 and more is planned in bug 1914502. Should this bug be duped over, or maybe the patches here can be reused?

Comment 5

[Jan de Mooij [:jandem]]

For Object.keys anba added some fast paths in bug 1406095. I think with that and the other bugs you mentioned we can close this one.