Created attachment 458644 [details] microbenchmark TypedArray.from(otherTypedArrayView) uses Symbol.iterator when it could use memmove() or possibly memcpy. A microbenchmark is attached that runs in both JSC shell and node. On macOS 12.3 aarch64 M1X For 1 MB: - JSC: 22ms - Node : 0.13ms For 32 MB: - JSC: 274ms - Node: 4ms If TypedArray.prototype.set[0] is used in TypedArrayConstructor.js when !mapFn && @isTypedArrayView(arrayLike), that becomes: For 1 MB: - JSC: 22ms - JSC (modified): 0.4ms - v8 9.6 (Node 17.7.1): 0.13ms For 32 MB: - JSC: 274ms - JSC (modified): 5ms - v8 9.6 (Node 17.7.1): 4ms It isn't precisely correct to use TypedArray.prototype.set, but it shows roughly what the numbers would look like after this is fixed. It does seem that V8 is consistently slightly faster after this change though, which means there is still something more to do here. Maybe it could allocate uninitialized memory because it will copy directly from the other typed array view? V8's optimization for TypedArray.from: https://github.com/v8/v8/blob/f32335fea75b7bde495e0800d7f7349253f81a7c/src/builtins/typed-array-from.tq#L167 [0]: https://gist.github.com/Jarred-Sumner/543b94142de9f17a9ec86e9dac5cf171#file-typedarrayconstructor-js
<rdar://problem/92917413>
Pull request: https://github.com/WebKit/WebKit/pull/564
Right condition to take this fast path is actually subtle :)
BTW, it seems that SpiderMonkey and V8 optimize this function incorrectly. The following test pass only in JSC (and from the spec, JSC's behavior is correct since we use ArrayIterator, which uses "length" property, not TypedArrayLength). function shouldBe(actual, expected) { if (actual !== expected) throw new Error('bad value: ' + actual); } var array = new Uint8Array(128); Reflect.defineProperty(array, 'length', { value: 42 }); var result = Uint8Array.from(array); shouldBe(result.length, 42);
Committed 252976@main (1b440efcb4ae): <https://commits.webkit.org/252976@main> Reviewed commits have been landed. Closing PR #564 and removing active labels.
(In reply to Yusuke Suzuki from comment #4) > BTW, it seems that SpiderMonkey and V8 optimize this function incorrectly. > The following test pass only in JSC (and from the spec, JSC's behavior is > correct since we use ArrayIterator, which uses "length" property, not > TypedArrayLength). It's actually the other way around. JSC is incorrectly using the "length" [1] property, whereas the spec mandates that for objects with a [[TypedArrayName]] internal slot, the [[ArrayLength]] internal slot is read [2]. :-) [1] https://github.com/WebKit/WebKit/blob/3f019cf4b5d2b381db5af9d2751583f7871ba8bf/Source/JavaScriptCore/builtins/ArrayIteratorPrototype.js#L53 [2] https://tc39.es/ecma262/#sec-createarrayiterator
(In reply to André Bargull from comment #6) > (In reply to Yusuke Suzuki from comment #4) > > BTW, it seems that SpiderMonkey and V8 optimize this function incorrectly. > > The following test pass only in JSC (and from the spec, JSC's behavior is > > correct since we use ArrayIterator, which uses "length" property, not > > TypedArrayLength). > > It's actually the other way around. JSC is incorrectly using the "length" > [1] property, whereas the spec mandates that for objects with a > [[TypedArrayName]] internal slot, the [[ArrayLength]] internal slot is read > [2]. :-) > > [1] > https://github.com/WebKit/WebKit/blob/ > 3f019cf4b5d2b381db5af9d2751583f7871ba8bf/Source/JavaScriptCore/builtins/ > ArrayIteratorPrototype.js#L53 > [2] https://tc39.es/ecma262/#sec-createarrayiterator Oh, interesting. We can make the implementation simpler.
(In reply to Yusuke Suzuki from comment #7) > (In reply to André Bargull from comment #6) > > (In reply to Yusuke Suzuki from comment #4) > > > BTW, it seems that SpiderMonkey and V8 optimize this function incorrectly. > > > The following test pass only in JSC (and from the spec, JSC's behavior is > > > correct since we use ArrayIterator, which uses "length" property, not > > > TypedArrayLength). > > > > It's actually the other way around. JSC is incorrectly using the "length" > > [1] property, whereas the spec mandates that for objects with a > > [[TypedArrayName]] internal slot, the [[ArrayLength]] internal slot is read > > [2]. :-) > > > > [1] > > https://github.com/WebKit/WebKit/blob/ > > 3f019cf4b5d2b381db5af9d2751583f7871ba8bf/Source/JavaScriptCore/builtins/ > > ArrayIteratorPrototype.js#L53 > > [2] https://tc39.es/ecma262/#sec-createarrayiterator > > Oh, interesting. We can make the implementation simpler. Will be fixed in https://github.com/WebKit/WebKit/pull/3038, it makes implementation simpler!