Align %TypedArray% constructor behavior with spec
Created attachment 415379 [details] Patch
Created attachment 415391 [details] Patch
Comment on attachment 415391 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=415391&action=review > Source/JavaScriptCore/ChangeLog:21 > + 3. Typed array argument (https://tc39.es/ecma262/#sec-initializetypedarrayfromtypedarray): > + We need to support the case where the input typed array uses a custom ArrayBuffer. > + This case is *extremely* strange -- we still create the same type of typed array with a normal ArrayBuffer, > + but we override the prototype of that ArrayBuffer to input.buffer.constructor[@@species].prototype. Note: There appears to be interest in removing @@species for ArrayBuffers, so it might be useful to have JSC continue to fail these tests as "web compatibility insurance". In that case, we could split (3) out from the rest.
Oops, I forgot that the relevant proposal about the @@species removal is https://github.com/tc39/proposal-rm-builtin-subclassing. But it sounds like folks would like to remove @@species from ArrayBuffers even if web compat issues were discovered for other classes.
Comment on attachment 415391 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=415391&action=review Looks good! But I think we should have species optimization at the first patch since new TypedArray(arrayBuffer) is common enough. > Source/JavaScriptCore/runtime/JSGenericTypedArrayViewConstructorInlines.h:115 > + JSValue constructor = source->get(globalObject, vm.propertyNames->constructor); Let's avoid doing property access by having watchpoint like Array / ArrayBuffer are doing since `new TypedArray(arrayBuffer)` is enough critical path. See speciesConstructArrayBuffer and speciesConstructArray.
Comment on attachment 415391 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=415391&action=review >> Source/JavaScriptCore/runtime/JSGenericTypedArrayViewConstructorInlines.h:115 >> + JSValue constructor = source->get(globalObject, vm.propertyNames->constructor); > > Let's avoid doing property access by having watchpoint like Array / ArrayBuffer are doing since `new TypedArray(arrayBuffer)` is enough critical path. > See speciesConstructArrayBuffer and speciesConstructArray. Possibly I think modifying speciesConstructArrayBuffer and reusing it would be possible.
Created attachment 415594 [details] Patch
Comment on attachment 415594 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=415594&action=review Nice, r=me > Source/JavaScriptCore/runtime/JSArrayBufferPrototype.cpp:68 > + VM& vm = globalObject->vm(); > + auto scope = DECLARE_THROW_SCOPE(vm); > + > + bool isValid = speciesWatchpointIsValid(vm, thisObject, mode); > + scope.assertNoException(); > + if (LIKELY(isValid)) > + return WTF::nullopt; > + Can you make this part inlined when the caller is calling this? (like, definition it in JSArrayBufferPrototypeInlines.h) I think 99.9999999% of path will go to this "watchpoint is valid" case. So it is worth inlining.
Created attachment 415661 [details] Patch for landing
Committed r270552: <https://trac.webkit.org/changeset/270552> All reviewed patches have been landed. Closing bug and clearing flags on attachment 415661 [details].
<rdar://problem/72106111>