Status

()

enhancement
P3
normal
2 years ago
2 years ago

People

(Reporter: evilpie, Unassigned)

Tracking

(Blocks 1 bug, {triage-deferred})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Reporter

Description

2 years ago
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.
Reporter

Comment 1

2 years ago
Posted patch WIPSplinter Review
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[]".
Reporter

Comment 2

2 years ago
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.
Reporter

Comment 3

2 years ago
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?
Reporter

Updated

2 years ago
Assignee: evilpies → nobody
Keywords: triage-deferred
Priority: -- → P3
You need to log in before you can comment on or make changes to this bug.