New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 225811 link

Starred by 8 users

Issue metadata

Status: Fixed
Owner:
Vacation until after Christmas
Closed: Oct 18
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Feature

Blocking:
issue v8:3533



Sign in to add a comment

Improve DataView performance

Project Member Reported by kbr@chromium.org, Apr 2 2013

Issue description

Version: 28.0.1460.0 (Official Build 191761) canary
OS: all

Currently, the DataView type from the typed array specification is implemented via the DOM bindings. Each read and write operation requires a call out to C++, which is far too much overhead.

The attached test case from the MapsGL team shows that implementing DataView-like classes in pure JavaScript using the typed array views, which are heavily optimized, is roughly 7 times faster than using DataView itself.

As part of the effort to implement typed arrays in the V8 VM, DataView's methods should be intrinsified. Each one should be a fairly small amount of IR in the compiler, and should be easily inlinable.

What steps will reproduce the problem?
1. Run the attached test case.

What is the expected output? What do you see instead?

Expect the DataView versions of the tests to be comparable in performance to the versions implemented in JavaScript using typed arrays. Instead, they are roughly 7x slower.

 
dataviewperf.html
573 bytes View Download
dataviewperf.js
4.9 KB View Download

Comment 1 by kbr@chromium.org, Apr 2 2013

Cc: bajones@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 5 2013

Labels: Cr-Blink
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 6 2013

Labels: -Cr-Content-JavaScript Cr-Blink-JavaScript
[Automated message] This open bug has seen not seen activity for over half a year and is not assigned to an owner. It seems it is not valid anymore. This issue will be set to Archived (Closed) next week. If you think this issue is still valid and should stay open, please contact hablich@.
Labels: -Type-Bug Type
Owner: jochen@chromium.org
Changing to feature as it seems to be a feature.

PTAL and decide if this is desirable to be implemented.

Comment 6 by jochen@chromium.org, Mar 20 2015

Owner: dslomov@chromium.org

Comment 7 by kbr@chromium.org, Mar 20 2015

Status: Assigned
It's definitely desirable. I haven't benchmarked Dmitry's new DataView implementation written in JavaScript; it may have addressed the majority of the performance issue.

Comment 8 by kbr@chromium.org, Mar 20 2015

Labels: -Type Type-Feature
Why is DataView so much slower than Uint8Array and friends?

On my machine, a compliant javascript implementation backed by several Typed Array Views is an order of magnitude faster than the platform's DataView! What makes its native implementation so different than the views?

Comment 10 Deleted

Labels: -Cr-Blink-JavaScript Cr-Blink-JavaScript-Language
Labels: Hotlist-Recharge
This issue likely requires triage.  The current issue owner may be inactive (i.e. hasn't fixed an issue in the last 30 days or commented in this particular issue in the last 90 days).  Thanks for helping out!

-Anthony
Owner: jochen@chromium.org
Jochen@ what should happen with this issue?

Current results for M49:
Test	DataView	JavaScript
Small Buffer Little Endian	281.31500000000005	23.319999999999993
Small Buffer Big Endian	278.29999999999995	35.89499999999998
Medium Buffer Little Endian	268.5050000000001	20.239999999999895
Medium Buffer Big Endian	262.16999999999996	20.220000000000027
Big Buffer Little Endian	272.2650000000001	25.529999999999973
Big Buffer Big Endian	275.905	27.0150000000001
Cc: jochen@chromium.org
Owner: ----
Status: Untriaged
DataView is implemented in V8 nowadays. I guess the speed difference comes from the indexed access being inlined in crankshaft, while the DataView methods are not.

I don't think we want to add custom inlined code to crankshaft for this at this point.

Comment 15 by kbr@chromium.org, Dec 8 2015

Cc: -dslomov@chromium.org mstarzinger@chromium.org titzer@chromium.org yangguo@chromium.org
We probably don't want to add custom inlined code for the DataView accessors themselves, but the primitives they are built on are pretty small: basically, unaligned and optionally byte-swapped loads and stores. Compare this to typed arrays, whose accesses are always aligned.

Could these be added as intrinsics to V8 and then the bounds checking, etc. be written in JavaScript?

Status: Available
Labels: -Cr-Blink
Removing Cr-Blink from issues that already have Cr-Blink sub-label set.

Comment 18 by travn...@gmail.com, May 29 2016

There is a need for fast DataView implementation. TypedArrays are good, but they do not allow packing of mixed data. Two points to consider:
 - cache coherence. When you use separate arrays for different "fields" of the same data structure - you lose out.
 - memory footprint. If you wish to squeeze out some use out of every byte of memory, being able to put Int32 right next to Uint8 is valuable.

I have 2 implementations of BVH (Bounding Volume Hierarchy), one using objects and building a tree of references in a style of typical javascript program, and another one using a single ArrayBuffer and a DataView. In terms of performance DataView implementation is over 2x slower, this should not be the case from my perspective.

I've attached a screenshot of profiling info for the DataView-based implementation, you will notice that V8 takes 60% of the times just on the DataView path.
Capture-DataView performance.PNG
64.2 KB View Download

Comment 19 by kbr@chromium.org, Jun 1 2016

Components: -Blink>JavaScript>Language Blink>JavaScript

Comment 20 Deleted

Comment 21 by fin...@gmail.com, Mar 23 2017

I create a project : https://github.com/finscn/FastDataView.js

In my project , the performance will be 10x faster.

Why is native DataView with poor performance ?
Project Member

Comment 22 by sheriffbot@chromium.org, May 9 2018

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 23 by kbr@chromium.org, May 9 2018

Cc: bmeu...@chromium.org mvstan...@chromium.org
Labels: -Hotlist-Recharge-Cold
Owner: mvstan...@chromium.org
Status: Assigned (was: Untriaged)
bmeurer, mvstanton: could we pick up this old bug and finally fix it?

The DataView methods should be straightforward to optimize – a bounds check, possibly an if-test for alignment (if deciding to use a different instruction sequence for unaligned loads/stores), and a machine instruction or a few for the actual load.

Thanks in advance for looking into this.

Cc: theotime@google.com
Having a look...
Status: Started (was: Assigned)
We landed a CL that turns Ken's test into a micro-benchmark. This will allow us to track how close we get to that ersatz TypedArray solution. (https://chromium.googlesource.com/v8/v8/+/c79feb8aa392ecfc0e51f6992dab0ef5b805aa2f).

Design doc here: https://docs.google.com/document/d/1Os3yomeIBXpRSYWk08yeBMc7efeBTU8oPFuJ8HUEyN0/edit?usp=sharing


Blocking: v8:3533
Project Member

Comment 27 by bugdroid1@chromium.org, Aug 13

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/9ae3e619b7fc7490b4ebaec0376f0fc8be0bf6a4

commit 9ae3e619b7fc7490b4ebaec0376f0fc8be0bf6a4
Author: Benedikt Meurer <bmeurer@chromium.org>
Date: Mon Aug 13 17:54:25 2018

[turbofan] Make use of the neutering protector for DataViews.

The DataView access methods can use the neutering protector to avoid
introducing an explicit check into the optimized code to see if the
backing store was neutered. Instead the optimized code has an implicit
dependency on the global neutering protector which gets invalidated
when the first array buffer is neutered (globally). We use the same
trick for typed arrays already.

Bug:  chromium:225811 
Change-Id: I9b3c95b3113b8fa00dcbba216ef29c84c0056951
Reviewed-on: https://chromium-review.googlesource.com/1172779
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55097}
[modify] https://crrev.com/9ae3e619b7fc7490b4ebaec0376f0fc8be0bf6a4/src/compiler/js-call-reducer.cc
[modify] https://crrev.com/9ae3e619b7fc7490b4ebaec0376f0fc8be0bf6a4/test/mjsunit/compiler/dataview-get.js

Project Member

Comment 28 by bugdroid1@chromium.org, Aug 13

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/c46915b931ed533ddfca1405660115e772eb7e70

commit c46915b931ed533ddfca1405660115e772eb7e70
Author: Benedikt Meurer <bmeurer@chromium.org>
Date: Mon Aug 13 19:23:28 2018

[turbofan] Further optimize DataView accesses.

This adds support for unaligned load/store access to the DataView
backing store and uses byteswap operations to fix up the endianess
when necessary. This changes the Word32ReverseBytes operator to be
a required operator and adds the missing support on the Intel and
ARM platforms (on 64-bit platforms the Word64ReverseBytes operator
is also mandatory now).

This further improves the performance on the dataviewperf.js test
mentioned in the tracking bug by up to 40%, and at the same time
reduces the code complexity in the EffectControlLinearizer.

Bug:  chromium:225811 
Change-Id: I296170b828c2ccc1c317ed37840b564aa14cdec2
Reviewed-on: https://chromium-review.googlesource.com/1172777
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55099}
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/arm/assembler-arm.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/arm/assembler-arm.h
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/arm/disasm-arm.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/arm/simulator-arm.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/arm64/macro-assembler-arm64-inl.h
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/arm64/macro-assembler-arm64.h
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/compiler/arm/code-generator-arm.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/compiler/arm/instruction-codes-arm.h
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/compiler/arm/instruction-scheduler-arm.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/compiler/arm/instruction-selector-arm.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/compiler/arm64/code-generator-arm64.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/compiler/arm64/instruction-codes-arm64.h
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/compiler/arm64/instruction-scheduler-arm64.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/compiler/arm64/instruction-selector-arm64.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/compiler/effect-control-linearizer.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/compiler/effect-control-linearizer.h
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/compiler/graph-assembler.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/compiler/graph-assembler.h
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/compiler/ia32/code-generator-ia32.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/compiler/ia32/instruction-codes-ia32.h
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/compiler/ia32/instruction-scheduler-ia32.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/compiler/ia32/instruction-selector-ia32.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/compiler/int64-lowering.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/compiler/machine-operator.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/compiler/machine-operator.h
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/compiler/memory-optimizer.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/compiler/raw-machine-assembler.h
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/compiler/wasm-compiler.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/compiler/x64/code-generator-x64.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/compiler/x64/instruction-codes-x64.h
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/compiler/x64/instruction-scheduler-x64.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/compiler/x64/instruction-selector-x64.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/ia32/assembler-ia32.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/ia32/assembler-ia32.h
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/ia32/disasm-ia32.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/x64/assembler-x64.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/x64/assembler-x64.h
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/src/x64/disasm-x64.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/test/cctest/compiler/test-run-machops.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/test/cctest/test-disasm-arm.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/test/cctest/test-disasm-ia32.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/test/cctest/test-disasm-x64.cc
[modify] https://crrev.com/c46915b931ed533ddfca1405660115e772eb7e70/test/unittests/compiler/int64-lowering-unittest.cc

Project Member

Comment 29 by bugdroid1@chromium.org, Aug 14

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/6c7c81e07ceb6638ce38cdf496e71082df44b225

commit 6c7c81e07ceb6638ce38cdf496e71082df44b225
Author: Benedikt Meurer <bmeurer@chromium.org>
Date: Tue Aug 14 07:45:04 2018

[turbofan] Properly zero-extend indices on 64-bit architectures.

This was an oversight from the previous CL. It doesn't really matter
with the current code generation pattern, since the upper bits of the
index will always be zero, but that might change in the future.

Bug:  chromium:225811 
Change-Id: I568a0824cad9ce9b73a56decc15d146c7dc675a1
Reviewed-on: https://chromium-review.googlesource.com/1174111
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55104}
[modify] https://crrev.com/6c7c81e07ceb6638ce38cdf496e71082df44b225/src/compiler/effect-control-linearizer.cc

Project Member

Comment 30 by bugdroid1@chromium.org, Aug 14

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e

commit 6a62d88e9b7ee9bafa169209684dc5c27f1fd58e
Author: Leszek Swirski <leszeks@chromium.org>
Date: Tue Aug 14 08:25:24 2018

Revert "[turbofan] Further optimize DataView accesses."

This reverts commit c46915b931ed533ddfca1405660115e772eb7e70.

Reason for revert: Disasm failures https://ci.chromium.org/p/v8/builders/luci.v8.ci/V8%20Linux%20-%20debug/21727 

Original change's description:
> [turbofan] Further optimize DataView accesses.
> 
> This adds support for unaligned load/store access to the DataView
> backing store and uses byteswap operations to fix up the endianess
> when necessary. This changes the Word32ReverseBytes operator to be
> a required operator and adds the missing support on the Intel and
> ARM platforms (on 64-bit platforms the Word64ReverseBytes operator
> is also mandatory now).
> 
> This further improves the performance on the dataviewperf.js test
> mentioned in the tracking bug by up to 40%, and at the same time
> reduces the code complexity in the EffectControlLinearizer.
> 
> Bug:  chromium:225811 
> Change-Id: I296170b828c2ccc1c317ed37840b564aa14cdec2
> Reviewed-on: https://chromium-review.googlesource.com/1172777
> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
> Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#55099}

TBR=sigurds@chromium.org,bmeurer@chromium.org

Change-Id: If7a62e3a1a4ad26823fcbd2ab6eb4c053ad11c49
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  chromium:225811 
Reviewed-on: https://chromium-review.googlesource.com/1174171
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55107}
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/arm/assembler-arm.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/arm/assembler-arm.h
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/arm/disasm-arm.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/arm/simulator-arm.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/arm64/macro-assembler-arm64-inl.h
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/arm64/macro-assembler-arm64.h
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/compiler/arm/code-generator-arm.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/compiler/arm/instruction-codes-arm.h
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/compiler/arm/instruction-scheduler-arm.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/compiler/arm/instruction-selector-arm.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/compiler/arm64/code-generator-arm64.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/compiler/arm64/instruction-codes-arm64.h
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/compiler/arm64/instruction-scheduler-arm64.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/compiler/arm64/instruction-selector-arm64.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/compiler/effect-control-linearizer.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/compiler/effect-control-linearizer.h
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/compiler/graph-assembler.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/compiler/graph-assembler.h
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/compiler/ia32/code-generator-ia32.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/compiler/ia32/instruction-codes-ia32.h
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/compiler/ia32/instruction-scheduler-ia32.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/compiler/ia32/instruction-selector-ia32.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/compiler/int64-lowering.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/compiler/machine-operator.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/compiler/machine-operator.h
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/compiler/memory-optimizer.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/compiler/raw-machine-assembler.h
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/compiler/wasm-compiler.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/compiler/x64/code-generator-x64.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/compiler/x64/instruction-codes-x64.h
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/compiler/x64/instruction-scheduler-x64.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/compiler/x64/instruction-selector-x64.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/ia32/assembler-ia32.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/ia32/assembler-ia32.h
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/ia32/disasm-ia32.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/x64/assembler-x64.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/x64/assembler-x64.h
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/src/x64/disasm-x64.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/test/cctest/compiler/test-run-machops.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/test/cctest/test-disasm-arm.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/test/cctest/test-disasm-ia32.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/test/cctest/test-disasm-x64.cc
[modify] https://crrev.com/6a62d88e9b7ee9bafa169209684dc5c27f1fd58e/test/unittests/compiler/int64-lowering-unittest.cc

Project Member

Comment 31 by bugdroid1@chromium.org, Aug 14

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/4c98815a191ad9253e1ca23e46a8c4d113599d9d

commit 4c98815a191ad9253e1ca23e46a8c4d113599d9d
Author: Benedikt Meurer <bmeurer@chromium.org>
Date: Tue Aug 14 08:29:09 2018

Revert "[turbofan] Properly zero-extend indices on 64-bit architectures."

This reverts commit 6c7c81e07ceb6638ce38cdf496e71082df44b225.

Reason for revert: Dependent CL was reverted.

Original change's description:
> [turbofan] Properly zero-extend indices on 64-bit architectures.
> 
> This was an oversight from the previous CL. It doesn't really matter
> with the current code generation pattern, since the upper bits of the
> index will always be zero, but that might change in the future.
> 
> Bug:  chromium:225811 
> Change-Id: I568a0824cad9ce9b73a56decc15d146c7dc675a1
> Reviewed-on: https://chromium-review.googlesource.com/1174111
> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
> Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#55104}

TBR=jarin@chromium.org,bmeurer@chromium.org

Change-Id: Ib344609b0c4734c6512e6be287a5b7f80bc3f603
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  chromium:225811 
Reviewed-on: https://chromium-review.googlesource.com/1174232
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55109}
[modify] https://crrev.com/4c98815a191ad9253e1ca23e46a8c4d113599d9d/src/compiler/effect-control-linearizer.cc

Project Member

Comment 32 by bugdroid1@chromium.org, Aug 14

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/5fecd146bfbea6edd3a92d52f61462b7455e8f3c

commit 5fecd146bfbea6edd3a92d52f61462b7455e8f3c
Author: Benedikt Meurer <bmeurer@chromium.org>
Date: Tue Aug 14 09:20:47 2018

[turbofan] Further optimize DataView accesses.

This adds support for unaligned load/store access to the DataView
backing store and uses byteswap operations to fix up the endianess
when necessary. This changes the Word32ReverseBytes operator to be
a required operator and adds the missing support on the Intel and
ARM platforms (on 64-bit platforms the Word64ReverseBytes operator
is also mandatory now).

This further improves the performance on the dataviewperf.js test
mentioned in the tracking bug by up to 40%, and at the same time
reduces the code complexity in the EffectControlLinearizer.

Bug:  chromium:225811 
Change-Id: I7c1ec826faf46a144a5a9068f8f815a5fd040997
Reviewed-on: https://chromium-review.googlesource.com/1174252
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55111}
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/arm/assembler-arm.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/arm/assembler-arm.h
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/arm/disasm-arm.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/arm/simulator-arm.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/arm64/macro-assembler-arm64-inl.h
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/arm64/macro-assembler-arm64.h
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/compiler/arm/code-generator-arm.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/compiler/arm/instruction-codes-arm.h
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/compiler/arm/instruction-scheduler-arm.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/compiler/arm/instruction-selector-arm.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/compiler/arm64/code-generator-arm64.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/compiler/arm64/instruction-codes-arm64.h
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/compiler/arm64/instruction-scheduler-arm64.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/compiler/arm64/instruction-selector-arm64.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/compiler/effect-control-linearizer.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/compiler/effect-control-linearizer.h
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/compiler/graph-assembler.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/compiler/graph-assembler.h
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/compiler/ia32/code-generator-ia32.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/compiler/ia32/instruction-codes-ia32.h
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/compiler/ia32/instruction-scheduler-ia32.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/compiler/ia32/instruction-selector-ia32.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/compiler/int64-lowering.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/compiler/machine-operator.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/compiler/machine-operator.h
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/compiler/memory-optimizer.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/compiler/raw-machine-assembler.h
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/compiler/wasm-compiler.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/compiler/x64/code-generator-x64.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/compiler/x64/instruction-codes-x64.h
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/compiler/x64/instruction-scheduler-x64.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/compiler/x64/instruction-selector-x64.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/ia32/assembler-ia32.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/ia32/assembler-ia32.h
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/ia32/disasm-ia32.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/x64/assembler-x64.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/x64/assembler-x64.h
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/src/x64/disasm-x64.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/test/cctest/compiler/test-run-machops.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/test/cctest/test-disasm-arm.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/test/cctest/test-disasm-ia32.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/test/cctest/test-disasm-x64.cc
[modify] https://crrev.com/5fecd146bfbea6edd3a92d52f61462b7455e8f3c/test/unittests/compiler/int64-lowering-unittest.cc

Project Member

Comment 33 by bugdroid1@chromium.org, Aug 17

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/5232b938d7113415bd469f6ef5bf270c4fb2313d

commit 5232b938d7113415bd469f6ef5bf270c4fb2313d
Author: Benedikt Meurer <bmeurer@chromium.org>
Date: Fri Aug 17 12:01:44 2018

[turbofan] Optimize index checking for DataView accesses.

Use CheckBounds and reduce the number of checks required to sanitize the
indices for DataView accesses in optimized code. Also constant-fold the
[[ByteLength]] if the DataView is a known compile-time constant (similar
to what we do for TypedArrays already). This further improves performance
of DataViews by 2-7% depending on the exact test case.

With this change DataView and TypedArray accesses themselves are mostly
on par performance wise.

Bug:  chromium:225811 
Change-Id: I6838339108b8a4dcf9b13ddecab40f1c3632967c
Reviewed-on: https://chromium-review.googlesource.com/1179741
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55190}
[modify] https://crrev.com/5232b938d7113415bd469f6ef5bf270c4fb2313d/src/compiler/js-call-reducer.cc

Project Member

Comment 34 by bugdroid1@chromium.org, Aug 20

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/4f5a6db0f86a5bfd8f412410923c56b741c2e243

commit 4f5a6db0f86a5bfd8f412410923c56b741c2e243
Author: Benedikt Meurer <bmeurer@chromium.org>
Date: Mon Aug 20 18:44:18 2018

Revert "[turbofan] Optimize index checking for DataView accesses."

This reverts commit 5232b938d7113415bd469f6ef5bf270c4fb2313d.

Reason for revert: Regresses performance on the JSTests bots

Original change's description:
> [turbofan] Optimize index checking for DataView accesses.
> 
> Use CheckBounds and reduce the number of checks required to sanitize the
> indices for DataView accesses in optimized code. Also constant-fold the
> [[ByteLength]] if the DataView is a known compile-time constant (similar
> to what we do for TypedArrays already). This further improves performance
> of DataViews by 2-7% depending on the exact test case.
> 
> With this change DataView and TypedArray accesses themselves are mostly
> on par performance wise.
> 
> Bug:  chromium:225811 
> Change-Id: I6838339108b8a4dcf9b13ddecab40f1c3632967c
> Reviewed-on: https://chromium-review.googlesource.com/1179741
> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
> Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
> Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#55190}

TBR=sigurds@chromium.org,bmeurer@chromium.org,mathias@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  chromium:225811 
Change-Id: I90547f91bab27127f57ba812194d3a3e3deb8ff7
Reviewed-on: https://chromium-review.googlesource.com/1179563
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55232}
[modify] https://crrev.com/4f5a6db0f86a5bfd8f412410923c56b741c2e243/src/compiler/js-call-reducer.cc

Project Member

Comment 35 by bugdroid1@chromium.org, Aug 23

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/69985ac52734c807456157c59ead62438b558f2c

commit 69985ac52734c807456157c59ead62438b558f2c
Author: Benedikt Meurer <bmeurer@chromium.org>
Date: Thu Aug 23 11:36:17 2018

[turbofan] Optimize index checking for DataView accesses.

Use CheckBounds and reduce the number of checks required to sanitize the
indices for DataView accesses in optimized code. Also constant-fold the
[[ByteLength]] if the DataView is a known compile-time constant (similar
to what we do for TypedArrays already). This further improves performance
of DataViews by 2-7% depending on the exact test case.

With this change DataView and TypedArray accesses themselves are mostly
on par performance wise.

Since this CL introduces proper CheckBounds for the DataViews, instead
of the hand-craftet bounds checks, it is expected to regress performance
when untrusted code mitigations are on, since DataViews are also guarded
in optimized now. Without untrusted code mitigations, there's no negative
performance impact.

Tbr: sigurds@chromium.org
Bug:  chromium:225811 , chromium:876005
Change-Id: I4a69f81124635c9ba2c7e4c2dc912e2fd601061a
Reviewed-on: https://chromium-review.googlesource.com/1186408
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55346}
[modify] https://crrev.com/69985ac52734c807456157c59ead62438b558f2c/src/compiler/js-call-reducer.cc

Project Member

Comment 36 by bugdroid1@chromium.org, Sep 19

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/46573e51d859fcc72489abc314caa197cace2594

commit 46573e51d859fcc72489abc314caa197cace2594
Author: Benedikt Meurer <bmeurer@chromium.org>
Date: Wed Sep 19 14:06:25 2018

[es2015] Introduce JSDataView::external_pointer.

This adds a new external_pointer field to every JSDataView instance
which points directly into the backing store at the given view's
byte_offset. This was the DataView performance is now almost on
par with the TypedArray performance for accessing aligned memory
(with appropriate endianess). This also serves as prepatory work
to enable full 64-bit addressing of DataView backing stores in
optimized code (soonish).

This change optimizes the bounds checking sequence in TurboFan in
such a way that it further improves the DataView set/get performance
by around 10%, almost closing the remaining gap between DataViews
and TypedArrays.

Drive-by-fix: Get rid of the code duplication around DataView inlining
in the JSCallReducer and have only a single bottleneck method now.

Bug:  chromium:225811 , v8:4153, v8:7881, v8:8171
Change-Id: I9118efd4d19e93f0e51c931a9bec1a56a0f4593e
Reviewed-on: https://chromium-review.googlesource.com/1231994
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56042}
[modify] https://crrev.com/46573e51d859fcc72489abc314caa197cace2594/src/builtins/builtins-dataview.cc
[modify] https://crrev.com/46573e51d859fcc72489abc314caa197cace2594/src/builtins/data-view.tq
[modify] https://crrev.com/46573e51d859fcc72489abc314caa197cace2594/src/code-stub-assembler.cc
[modify] https://crrev.com/46573e51d859fcc72489abc314caa197cace2594/src/code-stub-assembler.h
[modify] https://crrev.com/46573e51d859fcc72489abc314caa197cace2594/src/compiler/access-builder.cc
[modify] https://crrev.com/46573e51d859fcc72489abc314caa197cace2594/src/compiler/access-builder.h
[modify] https://crrev.com/46573e51d859fcc72489abc314caa197cace2594/src/compiler/js-call-reducer.cc
[modify] https://crrev.com/46573e51d859fcc72489abc314caa197cace2594/src/compiler/js-call-reducer.h
[modify] https://crrev.com/46573e51d859fcc72489abc314caa197cace2594/src/heap/concurrent-marking.cc
[modify] https://crrev.com/46573e51d859fcc72489abc314caa197cace2594/src/heap/objects-visiting.h
[modify] https://crrev.com/46573e51d859fcc72489abc314caa197cace2594/src/objects-body-descriptors-inl.h
[modify] https://crrev.com/46573e51d859fcc72489abc314caa197cace2594/src/objects.cc
[modify] https://crrev.com/46573e51d859fcc72489abc314caa197cace2594/src/objects/js-array-buffer-inl.h
[modify] https://crrev.com/46573e51d859fcc72489abc314caa197cace2594/src/objects/js-array-buffer.h
[modify] https://crrev.com/46573e51d859fcc72489abc314caa197cace2594/src/objects/map.h
[modify] https://crrev.com/46573e51d859fcc72489abc314caa197cace2594/src/snapshot/deserializer.cc
[modify] https://crrev.com/46573e51d859fcc72489abc314caa197cace2594/src/snapshot/serializer.cc

Project Member

Comment 37 by bugdroid1@chromium.org, Sep 20

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/ec21639857c66cbc288ddcc8d39a91b27b7124a8

commit ec21639857c66cbc288ddcc8d39a91b27b7124a8
Author: Michael Achenbach <machenbach@chromium.org>
Date: Thu Sep 20 06:31:33 2018

Revert "[es2015] Introduce JSDataView::external_pointer."

This reverts commit 46573e51d859fcc72489abc314caa197cace2594.

Reason for revert: Speculative revert for breaking chromium integration.

Might break gpu tests and linux debug:
https://ci.chromium.org/p/v8/builders/luci.v8.ci/Mac%20V8%20FYI%20Release%20(Intel)/2554

Also blocks the roll:
https://chromium-review.googlesource.com/c/chromium/src/+/1234328

Original change's description:
> [es2015] Introduce JSDataView::external_pointer.
> 
> This adds a new external_pointer field to every JSDataView instance
> which points directly into the backing store at the given view's
> byte_offset. This was the DataView performance is now almost on
> par with the TypedArray performance for accessing aligned memory
> (with appropriate endianess). This also serves as prepatory work
> to enable full 64-bit addressing of DataView backing stores in
> optimized code (soonish).
> 
> This change optimizes the bounds checking sequence in TurboFan in
> such a way that it further improves the DataView set/get performance
> by around 10%, almost closing the remaining gap between DataViews
> and TypedArrays.
> 
> Drive-by-fix: Get rid of the code duplication around DataView inlining
> in the JSCallReducer and have only a single bottleneck method now.
> 
> Bug:  chromium:225811 , v8:4153, v8:7881, v8:8171
> Change-Id: I9118efd4d19e93f0e51c931a9bec1a56a0f4593e
> Reviewed-on: https://chromium-review.googlesource.com/1231994
> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
> Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
> Reviewed-by: Yang Guo <yangguo@chromium.org>
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#56042}

TBR=yangguo@chromium.org,mlippautz@chromium.org,tebbi@chromium.org,bmeurer@chromium.org

Change-Id: I614a90043b1574b19936c37987db94806cac3bd7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  chromium:225811 , v8:4153, v8:7881, v8:8171
Reviewed-on: https://chromium-review.googlesource.com/1234417
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56059}
[modify] https://crrev.com/ec21639857c66cbc288ddcc8d39a91b27b7124a8/src/builtins/builtins-dataview.cc
[modify] https://crrev.com/ec21639857c66cbc288ddcc8d39a91b27b7124a8/src/builtins/data-view.tq
[modify] https://crrev.com/ec21639857c66cbc288ddcc8d39a91b27b7124a8/src/code-stub-assembler.cc
[modify] https://crrev.com/ec21639857c66cbc288ddcc8d39a91b27b7124a8/src/code-stub-assembler.h
[modify] https://crrev.com/ec21639857c66cbc288ddcc8d39a91b27b7124a8/src/compiler/access-builder.cc
[modify] https://crrev.com/ec21639857c66cbc288ddcc8d39a91b27b7124a8/src/compiler/access-builder.h
[modify] https://crrev.com/ec21639857c66cbc288ddcc8d39a91b27b7124a8/src/compiler/js-call-reducer.cc
[modify] https://crrev.com/ec21639857c66cbc288ddcc8d39a91b27b7124a8/src/compiler/js-call-reducer.h
[modify] https://crrev.com/ec21639857c66cbc288ddcc8d39a91b27b7124a8/src/heap/concurrent-marking.cc
[modify] https://crrev.com/ec21639857c66cbc288ddcc8d39a91b27b7124a8/src/heap/objects-visiting.h
[modify] https://crrev.com/ec21639857c66cbc288ddcc8d39a91b27b7124a8/src/objects-body-descriptors-inl.h
[modify] https://crrev.com/ec21639857c66cbc288ddcc8d39a91b27b7124a8/src/objects.cc
[modify] https://crrev.com/ec21639857c66cbc288ddcc8d39a91b27b7124a8/src/objects/js-array-buffer-inl.h
[modify] https://crrev.com/ec21639857c66cbc288ddcc8d39a91b27b7124a8/src/objects/js-array-buffer.h
[modify] https://crrev.com/ec21639857c66cbc288ddcc8d39a91b27b7124a8/src/objects/map.h
[modify] https://crrev.com/ec21639857c66cbc288ddcc8d39a91b27b7124a8/src/snapshot/deserializer.cc
[modify] https://crrev.com/ec21639857c66cbc288ddcc8d39a91b27b7124a8/src/snapshot/serializer.cc

Project Member

Comment 38 by bugdroid1@chromium.org, Sep 20

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/a50baa246e28934a4f46e2fe40ba9833c941c5d9

commit a50baa246e28934a4f46e2fe40ba9833c941c5d9
Author: Benedikt Meurer <bmeurer@chromium.org>
Date: Thu Sep 20 14:05:25 2018

[turbofan] Reduce DataView access code duplication.

Part of https://chromium-review.googlesource.com/1231994 that landed
earlier, but was reverted due to breakage. Landing this cleanup
separately instead.

Drive-by-fix: Also add test coverage for the cases that weren't covered
properly (according to the test coverage bot).

Bug:  chromium:225811 , v8:8015
Change-Id: I9c13ed5fcf0ba9e6b190489e15df86970eafdc13
Reviewed-on: https://chromium-review.googlesource.com/1236213
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56087}
[modify] https://crrev.com/a50baa246e28934a4f46e2fe40ba9833c941c5d9/src/compiler/js-call-reducer.cc
[modify] https://crrev.com/a50baa246e28934a4f46e2fe40ba9833c941c5d9/src/compiler/js-call-reducer.h
[add] https://crrev.com/a50baa246e28934a4f46e2fe40ba9833c941c5d9/test/mjsunit/compiler/dataview-constant.js
[add] https://crrev.com/a50baa246e28934a4f46e2fe40ba9833c941c5d9/test/mjsunit/compiler/dataview-neutered.js
[add] https://crrev.com/a50baa246e28934a4f46e2fe40ba9833c941c5d9/test/mjsunit/compiler/dataview-nonconstant.js

Owner: bmeu...@chromium.org
Hi Benedikt, I see you are still working on DataViews? I'd say close this bug if we've got the support we need, otherwise keep going! :)

Status: Fixed (was: Started)
I'd say we accomplished what we wanted to. :-)
Fantastic! Thanks for making these improvements; they make a big difference!

Project Member

Comment 42 by bugdroid1@chromium.org, Oct 29

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/15c31fe461f319e4378b5e5fdb2f29e62752d8c0

commit 15c31fe461f319e4378b5e5fdb2f29e62752d8c0
Author: Benedikt Meurer <bmeurer@chromium.org>
Date: Mon Oct 29 15:17:57 2018

[turbofan] Add support for huge DataViews.

This introduces Word64 support for the CheckBounds operator, which now
lowers to either CheckedUint32Bounds or CheckedUint64Bounds after the
representation selection. The right hand side of CheckBounds can now
be any positive safe integer on 64-bit architectures, whereas it remains
Unsigned31 for 32-bit architectures. We only use the extended Word64
support when the right hand side is outside the Unsigned31 range, so
for everything except DataViews this means that the performance should
remain the same. The typing rule for the CheckBounds operator was
updated to reflect this new behavior.

The CheckBounds with a right hand side outside the Unsigned31 range will
pass a new Signed64 feedback kind, which is handled with newly introduced
CheckedFloat64ToInt64 and CheckedTaggedToInt64 operators in representation
selection.

The JSCallReducer lowering for DataView getType()/setType() methods was
updated to not smi-check the [[ByteLength]] and [[ByteOffset]] anymore,
but instead just use the raw uintptr_t values and operate on any value
(for 64-bit architectures these fields can hold any positive safe
integer, for 32-bit architectures it's limited to Unsigned31 range as
before). This means that V8 can now handle huge DataViews fully, without
falling off a performance cliff.

This refactoring even gave us some performance improvements, on a simple
micro-benchmark just exercising different DataView accesses we go from

  testDataViewGetUint8: 796 ms.
  testDataViewGetUint16: 997 ms.
  testDataViewGetInt32: 994 ms.
  testDataViewGetFloat64: 997 ms.

to

  testDataViewGetUint8: 895 ms.
  testDataViewGetUint16: 889 ms.
  testDataViewGetInt32: 888 ms.
  testDataViewGetFloat64: 890 ms.

meaning we lost around 10% on the single byte case, but gained 10% across
the board for all the other element sizes.

Design-Document: http://bit.ly/turbofan-word64
Bug:  chromium:225811 , v8:4153, v8:7881, v8:8171,  v8:8383 
Change-Id: Ic9d1bf152e47802c04dcfd679372e5c85e4abc83
Reviewed-on: https://chromium-review.googlesource.com/c/1303732
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57095}
[modify] https://crrev.com/15c31fe461f319e4378b5e5fdb2f29e62752d8c0/src/compiler/arm64/instruction-selector-arm64.cc
[modify] https://crrev.com/15c31fe461f319e4378b5e5fdb2f29e62752d8c0/src/compiler/effect-control-linearizer.cc
[modify] https://crrev.com/15c31fe461f319e4378b5e5fdb2f29e62752d8c0/src/compiler/effect-control-linearizer.h
[modify] https://crrev.com/15c31fe461f319e4378b5e5fdb2f29e62752d8c0/src/compiler/graph-assembler.h
[modify] https://crrev.com/15c31fe461f319e4378b5e5fdb2f29e62752d8c0/src/compiler/instruction-selector.cc
[modify] https://crrev.com/15c31fe461f319e4378b5e5fdb2f29e62752d8c0/src/compiler/js-call-reducer.cc
[modify] https://crrev.com/15c31fe461f319e4378b5e5fdb2f29e62752d8c0/src/compiler/machine-operator.cc
[modify] https://crrev.com/15c31fe461f319e4378b5e5fdb2f29e62752d8c0/src/compiler/machine-operator.h
[modify] https://crrev.com/15c31fe461f319e4378b5e5fdb2f29e62752d8c0/src/compiler/opcodes.h
[modify] https://crrev.com/15c31fe461f319e4378b5e5fdb2f29e62752d8c0/src/compiler/operation-typer.cc
[modify] https://crrev.com/15c31fe461f319e4378b5e5fdb2f29e62752d8c0/src/compiler/redundancy-elimination.cc
[modify] https://crrev.com/15c31fe461f319e4378b5e5fdb2f29e62752d8c0/src/compiler/representation-change.cc
[modify] https://crrev.com/15c31fe461f319e4378b5e5fdb2f29e62752d8c0/src/compiler/representation-change.h
[modify] https://crrev.com/15c31fe461f319e4378b5e5fdb2f29e62752d8c0/src/compiler/simplified-lowering.cc
[modify] https://crrev.com/15c31fe461f319e4378b5e5fdb2f29e62752d8c0/src/compiler/simplified-operator.cc
[modify] https://crrev.com/15c31fe461f319e4378b5e5fdb2f29e62752d8c0/src/compiler/simplified-operator.h
[modify] https://crrev.com/15c31fe461f319e4378b5e5fdb2f29e62752d8c0/src/compiler/verifier.cc
[modify] https://crrev.com/15c31fe461f319e4378b5e5fdb2f29e62752d8c0/src/compiler/x64/instruction-selector-x64.cc
[modify] https://crrev.com/15c31fe461f319e4378b5e5fdb2f29e62752d8c0/test/cctest/compiler/test-representation-change.cc
[modify] https://crrev.com/15c31fe461f319e4378b5e5fdb2f29e62752d8c0/test/mjsunit/compiler/int64.js
[modify] https://crrev.com/15c31fe461f319e4378b5e5fdb2f29e62752d8c0/test/unittests/compiler/redundancy-elimination-unittest.cc

Sign in to add a comment