Open
Bug 1410695
Opened 7 years ago
Updated 2 years ago
Consider adding lookup cache for some case in native code.
Categories
(Core :: JavaScript Engine, enhancement, P3)
Core
JavaScript Engine
Tracking
()
ASSIGNED
People
(Reporter: arai, Assigned: arai)
References
(Blocks 2 open bugs)
Details
Attachments
(6 files, 1 obsolete file)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
(derived from bug 1393712 comment #18)
These days we do something like following in several places in native code,
to detect optimizable case and enter optimized path after that.
like, SpeciesConstructor's case.
GetPropertyPure(cx, OBJ, ..., &value)
if (SomeCondition(value)) {
// optimizable
} else {
// not optimizable
}
there, OBJ is supposed to have same shape every time.
we do LookupPropertyPure every time inside GetPropertyPure,
but we could cache the shape and slot, and skip lookup.
Comment 1•7 years ago
|
||
For that matter, if there were a way to use ICs in places where native code calls GetProperty etc., we could speed *lots* of things up. Things like the interpreter.
Updated•7 years ago
|
Priority: -- → P3
Assignee | ||
Comment 2•7 years ago
|
||
based on bug 1317481.
performance comparison for bug 1393712 testcase.
before: ~440 [ms]
after: ~400 [ms]
(this will need to clear cache on GC, and also store cache per context or compartment tho...)
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•7 years ago
|
||
if we use cache per each callsite, the issue is where to store the cache.
we could store them into context or compartment, but it will increase the size of them regardless of the usage.
Assignee | ||
Updated•7 years ago
|
Assignee | ||
Comment 4•7 years ago
|
||
so far I found 2 cases:
1. want to check if the given object has given property with same value
2. want to check if the given object never has given property (bug 1412206)
I think, the case 2 can also be covered with the same way (checking shape for object and prototypes).
Assignee | ||
Comment 5•7 years ago
|
||
moved the cache from static variable to runtime
Attachment #8921429 -
Attachment is obsolete: true
Assignee | ||
Comment 6•7 years ago
|
||
To make it easier to use this cache system in other place, it would be nice to integrate cache system into function like GetProperty function or similar,
I'll check if it's possible to separate the conditions for each access in this case (currently the cache is updated at once when the whole conditions are met).
Assignee | ||
Comment 7•7 years ago
|
||
now checking how many times lookup happens in ARES6,
and looks like we're calling functions, that does LookupOwnPropertyInline inside, more than 1000 times with same shape and same property id in some callsites, with same result (both found and not found cases).
those will benefit from cache, as long as cache can build with faster way than shape.
Here's the reverse call graph and the shape/id/found for each path, where it hit more than 1000 times.
(it's partial execution of ARES-6 Air, not yet finished entire run)
bool js::LookupOwnPropertyInline<(js::AllowGC)1>(JSContext*, js::MaybeRooted<js::NativeObject*, (js::AllowGC)1>::HandleType, js::MaybeRooted<jsid, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::PropertyResult, (js::AllowGC)1>::MutableHandleType, bool*)
+ bool NativeGetPropertyInline<(js::AllowGC)1>(JSContext*, js::MaybeRooted<js::NativeObject*, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::Value, (js::AllowGC)1>::HandleType, js::MaybeRooted<jsid, (js::AllowGC)1>::HandleType, IsNameLookup, js::MaybeRooted<JS::Value, (js::AllowGC)1>::MutableHandleType)
| + js::NativeGetProperty(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<JS::Value>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>)
| + js::GetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>)
| + js::GetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, js::PropertyName*, JS::MutableHandle<JS::Value>)
| + js::GetProperty(JSContext*, JS::Handle<JS::Value>, JS::Handle<js::PropertyName*>, JS::MutableHandle<JS::Value>)
| + GetPropertyOperation(JSContext*, js::InterpreterFrame*, JS::Handle<JSScript*>, unsigned char*, JS::MutableHandle<JS::Value>, JS::MutableHandle<JS::Value>)
| + Interpret(JSContext*, js::RunState&) + 35118
| + js::RunScript(JSContext*, js::RunState&)
| + shape=0x10641b560, id=args => not found: 8827
| + shape=0x106402200, id=push => not found: 14681
| + shape=0x10641b268, id=add => not found: 1228
+ bool js::NativeLookupOwnProperty<(js::AllowGC)1>(JSContext*, js::MaybeRooted<js::NativeObject*, (js::AllowGC)1>::HandleType, js::MaybeRooted<jsid, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::PropertyResult, (js::AllowGC)1>::MutableHandleType)
+ js::NativeGetOwnPropertyDescriptor(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<jsid>, JS::MutableHandle<JS::PropertyDescriptor>)
| + js::GetOwnPropertyDescriptor(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JS::PropertyDescriptor>)
| + js::ScriptedProxyHandler::get(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>) const
| + js::Proxy::getInternal(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>)
| + js::ProxyGetPropertyByValue(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>)
| + 0x0
| + shape=0x106402200, id=[number] => found: 1181
+ js::NativeDefineProperty(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<jsid>, JS::Handle<JS::PropertyDescriptor>, JS::ObjectOpResult&)
+ js::NativeDefineDataProperty(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, unsigned int, JS::ObjectOpResult&)
| + js::NativeDefineDataProperty(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, unsigned int)
| + js::frontend::BytecodeEmitter::emitPropertyList(js::frontend::ParseNode*, JS::MutableHandle<js::PlainObject*>, js::frontend::PropListType)
| + js::frontend::BytecodeEmitter::emitObject(js::frontend::ParseNode*)
| + js::frontend::BytecodeEmitter::emitTree(js::frontend::ParseNode*, js::frontend::ValueUsage, js::frontend::BytecodeEmitter::EmitLineNumberNote)
| + js::frontend::BytecodeEmitter::emitCallOrNew(js::frontend::ParseNode*, js::frontend::ValueUsage)
| + shape=0x106406fb0, id=type => not found: 1302
| + shape=0x106404a10, id=role => not found: 1302
| + shape=0x106406fd8, id=width => not found: 1302
+ js::DefineAccessorProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, bool (*)(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>), bool (*)(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::ObjectOpResult&), unsigned int, JS::ObjectOpResult&)
| + js::DefineAccessorProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, bool (*)(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>), bool (*)(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::ObjectOpResult&), unsigned int)
| + js::InitGetterSetterOperation(JSContext*, unsigned char*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JSObject*>)
| + js::InitGetterSetterOperation(JSContext*, unsigned char*, JS::Handle<JSObject*>, JS::Handle<js::PropertyName*>, JS::Handle<JSObject*>)
| + 0x0
| + shape=0x105cf9d80, id=liveSet => not found: 1026
+ js::DefineDataProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, unsigned int, JS::ObjectOpResult&)
+ js::DefineDataProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, unsigned int)
+ js::DefineDataProperty(JSContext*, JS::Handle<JSObject*>, js::PropertyName*, JS::Handle<JS::Value>, unsigned int)
| + ResolveInterpretedFunctionPrototype(JSContext*, JS::Handle<JSFunction*>, JS::Handle<jsid>)
| + fun_resolve(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, bool*)
| + js::CallResolveOp(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<jsid>, JS::MutableHandle<JS::PropertyResult>, bool*)
| + shape=0x105cc7d08, id=constructor => not found: 1048
+ js::DefineDataElement(JSContext*, JS::Handle<JSObject*>, unsigned int, JS::Handle<JS::Value>, unsigned int)
+ js::InitArrayElemOperation(JSContext*, unsigned char*, JS::Handle<JSObject*>, unsigned int, JS::Handle<JS::Value>)
+ Interpret(JSContext*, js::RunState&) + 71707
+ js::RunScript(JSContext*, js::RunState&)
+ shape=0x105cb3040, id=[number] => not found: 2383
bool js::LookupOwnPropertyInline<(js::AllowGC)0>(JSContext*, js::MaybeRooted<js::NativeObject*, (js::AllowGC)0>::HandleType, js::MaybeRooted<jsid, (js::AllowGC)0>::HandleType, js::MaybeRooted<JS::PropertyResult, (js::AllowGC)0>::MutableHandleType, bool*)
bool js::LookupPropertyInline<(js::AllowGC)0>(JSContext*, js::MaybeRooted<js::NativeObject*, (js::AllowGC)0>::HandleType, js::MaybeRooted<jsid, (js::AllowGC)0>::HandleType, js::MaybeRooted<JSObject*, (js::AllowGC)0>::MutableHandleType, js::MaybeRooted<JS::PropertyResult, (js::AllowGC)0>::MutableHandleType)
+ js::LookupNameNoGC(JSContext*, js::PropertyName*, JSObject*, JSObject**, JSObject**, JS::PropertyResult*)
+ bool js::GetEnvironmentName<(js::GetNameMode)0>(JSContext*, JS::Handle<JSObject*>, JS::Handle<js::PropertyName*>, JS::MutableHandle<JS::Value>)
+ GetNameOperation(JSContext*, js::InterpreterFrame*, unsigned char*, JS::MutableHandle<JS::Value>)
+ Interpret(JSContext*, js::RunState&) + 48564
+ js::RunScript(JSContext*, js::RunState&)
+ js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct)
+ InternalCall(JSContext*, js::AnyInvokeArgs const&)
+ shape=0x105cf9858, id=Set => not found: 1054
+ shape=0x105cc7060, id=Set => not found: 1054
+ shape=0x105ce09e8, id=Set => not found: 1054
Assignee | ||
Comment 8•7 years ago
|
||
here's the entire result for ARES-6, where lookup happens for same shape and same property, and same result, over 1000 times.
I'm going to prototype cache system based on this result.
Assignee | ||
Comment 9•7 years ago
|
||
fwiw, here's the patch I used to log lookup
Assignee | ||
Comment 10•7 years ago
|
||
and script to parse the result
Assignee | ||
Comment 11•7 years ago
|
||
apparently, some functions are not shown in the result, because it's inlined :P
Assignee | ||
Comment 12•7 years ago
|
||
applied lookup cache to OrdinaryToPrimitive (it stores the cache to static tho).
here's the performance comparison of ARES-6 ML
[before]
firstIteration: 284.90 +- 2.85 ms
averageWorstCase: 239.93 +- 2.76 ms
steadyState: 226.02 +- 2.36 ms
summary: 249.06 +- 2.38 ms
[after]
firstIteration: 283.00 +- 3.90 ms
averageWorstCase: 238.65 +- 2.70 ms
steadyState: 224.07 +- 1.86 ms
summary: 247.34 +- 2.04 ms
Assignee | ||
Comment 13•7 years ago
|
||
Interestingly, just adding lookup cache without info about callsite context improves performance :3
[before]
firstIteration: 285.10 +- 7.63 ms
averageWorstCase: 240.10 +- 5.32 ms
steadyState: 224.08 +- 2.38 ms
summary: 248.39 +- 2.34 ms
[after]
firstIteration: 278.60 +- 5.90 ms
averageWorstCase: 234.68 +- 4.31 ms
steadyState: 221.72 +- 3.48 ms
summary: 243.82 +- 4.28 ms
Assignee | ||
Comment 14•7 years ago
|
||
in above cases, I'm mainly focusing on the following case:
1. object is an instance of builtin class
2. it's accessing prototype property
3. the direct prototype (obj->staticPrototype()) of the object has the property
that way, it can skip lookup on the object itself and prototype of it.
Assignee | ||
Comment 15•7 years ago
|
||
result with all tests (above were running only ML)
| before | after
--------+------------------+--------------------+--------------------
Air | firstIteration | 101.83 +- 2.24 ms | 102.83 +- 1.68 ms
| averageWorstCase | 60.13 +- 2.44 ms | 64.08 +- 7.46 ms
| steadyState | 18.80 +- 0.15 ms | 18.98 +- 0.34 ms
--------+------------------+--------------------+--------------------
Basic | firstIteration | 81.67 +- 1.58 ms | 83.00 +- 2.30 ms
| averageWorstCase | 71.42 +- 8.37 ms | 67.25 +- 6.83 ms
| steadyState | 52.81 +- 0.32 ms | 52.34 +- 0.30 ms
--------+------------------+--------------------+--------------------
Babylon | firstIteration | 116.33 +- 4.72 ms | 119.00 +- 6.33 ms
| averageWorstCase | 34.25 +- 2.39 ms | 33.21 +- 0.87 ms
| steadyState | 7.60 +- 0.13 ms | 7.67 +- 0.19 ms
--------+------------------+--------------------+--------------------
ML | firstIteration | 293.83 +- 17.50 ms | 288.67 +- 3.91 ms
| averageWorstCase | 250.92 +- 15.89 ms | 238.46 +- 4.12 ms
| steadyState | 225.95 +- 7.98 ms | 223.55 +- 3.70 ms
--------+------------------+--------------------+--------------------
summary | 71.44 +- 1.37 ms | 71.16 +- 0.94 ms
I'll check other places that lookup cache can be used.
Assignee | ||
Comment 16•7 years ago
|
||
I forgot that in most case the lookup fails.
the length of prototype chain | not found
------------------------------+-----------
0 | 13229
1 | 46536
2 | 26411
3 | 220
4 | 10136
I'll apply lookup cache for not found case and see if it works.
Comment 17•7 years ago
|
||
This is exciting work! I am looking forward to more results. I feel like we definitely want to have the cache per call-site, because different GetProperty calls probably have very different objects that they are usually called on. Maybe there would be some way to re-purpose CacheIR for this as well, writing IC can be quite error prone and this way we would immediately get huge coverage.
Assignee | ||
Comment 18•7 years ago
|
||
memo for myself
look into the interaction with dictionary mode.
http://searchfox.org/mozilla-central/rev/ed212c79cfe86357e9a5740082b9364e7f6e526f/js/src/vm/NativeObject.h#913
Comment 19•7 years ago
|
||
status-firefox59:
--- → ?
Updated•2 years ago
|
Severity: minor → S4
You need to log in
before you can comment on or make changes to this bug.
Description
•