bindings: Improve fast-path optimizations for JS -> WebIDL sequence conversion |
|||
Issue descriptionThis was spun off issue 690428 , specifically https://bugs.chromium.org/p/chromium/issues/detail?id=690428#c11. That issue was supposed to cover updating our JS-sequence conversion code and make it work with the latest WebIDL spec, in that we now support custom iterators and no longer have a special case for deprecated WebIDL arrays. That has been done, but simply converting all JS objects (including arrays) to sequences via https://heycam.github.io/webidl/#create-sequence-from-iterable is too slow (see https://bugs.webkit.org/show_bug.cgi?id=166834#c13, for example), so at least Gecko and WebKit implement some fast-path optimizations for e.g. arrays with no custom iterators or weird properties. We're lagging behind here, as the work in issue 690428 was focused on supporting the new sequence conversion steps while leaving the previous "optimization" path intact. This is problematic, as it amounts to basically calling v8::Value::IsArray() and iterating over it in a loop. We also use this path even if an array has a custom @@iterator symbol defined. We need to take a lot more into consideration when deciding whether to take shortcuts in the sequence conversion steps. For example, https://github.com/w3c/web-platform-tests/issues/5566#issuecomment-294181173 says "One thing that needs to be done carefully there (apart from not optimizing cases which have non-default iterator behavior) is either not optimizing arrays with accessors or handling changes to %ArrayIterator%.prototype.next partway through the iteration, when a getter for an accessor property on the array mutates it". Fixing this probably needs some work in V8 as well, or we risk being as slow as the code path for custom iterators we currently implement in the bindings code (it would also be great if V8 could do some of the work we currently do in NativeValueTraits<IDLSequence<T>>::ConvertSequenceSlow(), https://cs.chromium.org/chromium/src/third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h?type=cs&q=nativevaluetraitsimpl.h+package:%5Echromium$&l=343).
,
Apr 25 2017
Your analysis totally sounds reasonable to me.
,
May 10 2017
I wrote up a test suite in https://github.com/w3c/web-platform-tests/pull/5881; Chrome Canary fails most of them. These tests will be in-trunk under external/wpt/WebIDL/ecmascript-binding/sequence-conversion.html once they are approved and the next test-import rolls.
,
May 11 2017
Thanks domenic@, it makes it clear what we should implement. rakuco@, I assign this issue to you, but feel free to unassign yourself if you're not available, and also feel free to assign it back to me. This is P3 and not urgent.
,
May 11 2017
We aren't able to do much without involving the V8 folks; what's the best way to reach out to them to discuss this?
,
May 11 2017
jochen@ and Toon (verwaest@) are the right persons to talk to, and they're already cc'ed on this issue. I think that you can start the discussion here or create an e-mail thread for the discussion separately.
,
May 24 2017
jochen/toon, are you there? :-)
,
Nov 30
|
|||
►
Sign in to add a comment |
|||
Comment 1 by raphael....@intel.com
, Apr 25 2017