Bug 243148 - TypedArray speciesConstruct should do the same thing to Array's species constructor optimization in C++
Summary: TypedArray speciesConstruct should do the same thing to Array's species const...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: Safari 15
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
: 243150 (view as bug list)
Depends on:
Blocks:
 
Reported: 2022-07-23 21:03 PDT by SheetJS
Modified: 2022-07-26 19:29 PDT (History)
3 users (show)

See Also:


Attachments
v8 (1.37 MB, image/png)
2022-07-23 22:00 PDT, Jarred Sumner
no flags Details
jsc (1.57 MB, image/png)
2022-07-23 22:01 PDT, Jarred Sumner
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
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.