Bug 243148

Summary:

TypedArray speciesConstruct should do the same thing to Array's species constructor optimization in C++

Product:

WebKit

Reporter:

SheetJS <dev>

Component:

JavaScriptCore

Assignee:

Yusuke Suzuki <ysuzuki>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

jarred, webkit-bug-importer, ysuzuki

Priority:

Keywords:

InRadar

Version:

Safari 15

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
v8	none
jsc	none

Description SheetJS 2022-07-23 21:03:01 PDT

Parts of the SheetJS library will create many small slices from Uint8Arrays.  A typical use case is performing a shallow parse of a protobuf message (https://github.com/SheetJS/sheetjs/blob/master/modules/83_numbers.ts#L145 for example)

In V8, changing `buf.slice(start, end)` to `buf.subarray(start, end)` results in nearly identical performance.  If we had to speculate, when code only reads from the result, `slice` does not create a new copy of the data.

In Safari, changing `buf.slice(start, end)` to `buf.subarray(start, end)` results in a significant difference in running time. If we had to speculate, Safari is creating new copies with each slice even if the individual slices are not modified.

The performance difference is notable: ~20% savings when testing in the Bun runtime. It shaved nearly 80 seconds (from 453 to 373 seconds) from the browser test suite run time in iOS 11.3.

Given the evolution of `Buffer` in NodeJS and V8's performance profile, many isomorphic libraries use `slice` when `subarray` would have sufficed.

Comment 1 Jarred Sumner 2022-07-23 22:00:19 PDT

Created attachment 461170 [details]
v8

instruments output of v8 shell when running this code

var typedArray = new Uint8Array(1024);
typedArray.fill(253);
var output = typedArray.slice();
for (let i = 0; i < 10000000; i++) {
  output = output.slice();
}

Comment 2 Jarred Sumner 2022-07-23 22:01:45 PDT

Created attachment 461171 [details]
jsc

jsc shell output

Comment 3 Yusuke Suzuki 2022-07-23 22:05:35 PDT

It seems that V8's TypedArray#slice is straight forward, and I think we are doing similar things. https://source.chromium.org/chromium/chromium/src/+/main:v8/src/builtins/typed-array-slice.tq?q=typed-array-slice.tq&ss=chromium

Comment 4 Yusuke Suzuki 2022-07-23 22:26:25 PDT

TypedArray's speciesConstructor is always getting "constructor" from the object, that's bad.
We should have a watchpoint for species constructor invalidation for TypedArray, and let's skip this completely, as we are doing it in Array species constructor.

Comment 5 Yusuke Suzuki 2022-07-24 02:06:35 PDT

*** Bug 243150 has been marked as a duplicate of this bug. ***

Comment 6 Yusuke Suzuki 2022-07-24 02:07:13 PDT

Pull request: https://github.com/WebKit/WebKit/pull/2694

Comment 7 EWS 2022-07-26 15:34:22 PDT

Committed 252847@main (b4e6f9389bc5): <https://commits.webkit.org/252847@main>

Reviewed commits have been landed. Closing PR #2694 and removing active labels.

Comment 8 Radar WebKit Bug Importer 2022-07-26 15:35:18 PDT

<rdar://problem/97632409>

Comment 9 Yusuke Suzuki 2022-07-26 19:29:05 PDT

Now, the benchmark script is 2x faster than V8.