Bug 185003 – We should have a CoW storage for NewArrayBuffer arrays.

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Keith Miller]

...

[Radar WebKit Bug Importer]

<rdar://problem/39737329>

[Keith Miller]

Created attachment 340710 [details]
Patch

[Keith Miller]

Created attachment 340771 [details]
Patch

[Keith Miller]

Created attachment 340776 [details]
Patch

[EWS Watchlist]

Attachment 340776 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/Butterfly.h:57:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArrayMode.h:519:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/JavaScriptCore/dfg/DFGArrayMode.h:520:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/JavaScriptCore/runtime/JSObject.h:827:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/JSObject.h:839:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/JSObject.h:849:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/ButterflyInlines.h:37:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ButterflyInlines.h:39:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/JSObject.cpp:1552:  Extra space after ( in if  [whitespace/parens] [5]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:71:  Bad include order. Mixing system and custom headers.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:57:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/bytecode/ArrayProfile.h:60:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
Total errors found: 12 in 75 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 340776 [details]
Patch

Attachment 340776 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/7736815

New failing tests:
js/dfg-new-array-buffer-while-having-a-bad-time.html
js/slow-stress/variadic-closure-call.html

[EWS Watchlist]

Created attachment 340778 [details]
Archive of layout-test-results from ews101 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 340776 [details]
Patch

Attachment 340776 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/7736827

New failing tests:
js/dfg-new-array-buffer-while-having-a-bad-time.html
js/slow-stress/variadic-closure-call.html

[EWS Watchlist]

Created attachment 340780 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 340776 [details]
Patch

Attachment 340776 [details] did not pass jsc-ews (mac):
Output: http://webkit-queues.webkit.org/results/7736808

New failing tests:
stress/put-by-val-direct-out-of-bounds-setter.js.ftl-no-cjit-no-inline-validate
stress/put-by-val-direct-out-of-bounds-setter.js.no-cjit-validate-phases
stress/put-by-val-direct-out-of-bounds-setter.js.dfg-eager
stress/put-by-val-direct-out-of-bounds-setter.js.ftl-no-cjit-b3o1
stress/put-by-val-direct-out-of-bounds-setter.js.ftl-no-cjit-small-pool
jsc-layout-tests.yaml/js/script-tests/dfg-new-array-buffer-while-having-a-bad-time.js.layout-no-ftl
jsc-layout-tests.yaml/js/script-tests/dfg-new-array-buffer-while-having-a-bad-time.js.layout
stress/put-by-val-direct-out-of-bounds-setter.js.ftl-no-cjit-no-put-stack-validate
stress/destructuring-rest-element.js.ftl-eager
jsc-layout-tests.yaml/js/slow-stress/script-tests/variadic-closure-call.js.ftl-no-cjit-validate-sampling-profiler
jsc-layout-tests.yaml/js/slow-stress/script-tests/variadic-closure-call.js.no-cjit
jsc-layout-tests.yaml/js/script-tests/dfg-new-array-buffer-while-having-a-bad-time.js.layout-no-llint
stress/put-by-val-direct-out-of-bounds-setter.js.ftl-eager-no-cjit
jsc-layout-tests.yaml/js/script-tests/dfg-new-array-buffer-while-having-a-bad-time.js.layout-ftl-eager-no-cjit
jsc-layout-tests.yaml/js/slow-stress/script-tests/variadic-closure-call.js.default
jsc-layout-tests.yaml/js/slow-stress/script-tests/variadic-closure-call.js.no-ftl
stress/put-by-val-direct-out-of-bounds-setter.js.ftl-eager-no-cjit-b3o1
stress/put-by-val-direct-out-of-bounds-setter.js.ftl-eager
jsc-layout-tests.yaml/js/script-tests/dfg-new-array-buffer-while-having-a-bad-time.js.layout-no-cjit
stress/put-by-val-direct-out-of-bounds-setter.js.no-cjit-collect-continuously
jsc-layout-tests.yaml/js/script-tests/dfg-new-array-buffer-while-having-a-bad-time.js.layout-dfg-eager-no-cjit
stress/put-by-val-direct-out-of-bounds-setter.js.dfg-eager-no-cjit-validate
jsc-layout-tests.yaml/js/script-tests/dfg-new-array-buffer-while-having-a-bad-time.js.layout-ftl-no-cjit
stress/put-by-val-direct-out-of-bounds-setter.js.no-ftl
stress/put-by-val-direct-out-of-bounds-setter.js.default
stress/put-by-val-direct-out-of-bounds-setter.js.no-llint
stress/put-by-val-direct-out-of-bounds-setter.js.ftl-no-cjit-validate-sampling-profiler

[EWS Watchlist]

Comment on attachment 340776 [details]
Patch

Attachment 340776 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/7736861

New failing tests:
js/dfg-new-array-buffer-while-having-a-bad-time.html
js/slow-stress/variadic-closure-call.html

[EWS Watchlist]

Created attachment 340781 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4

[EWS Watchlist]

Comment on attachment 340776 [details]
Patch

Attachment 340776 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/7736971

New failing tests:
media/modern-media-controls/start-support/start-support-disable-controls-and-re-enable-post-play.html
js/dfg-new-array-buffer-while-having-a-bad-time.html
js/slow-stress/variadic-closure-call.html
inspector/canvas/recording-2d.html
webgl/1.0.2/conformance/more/conformance/quickCheckAPI-C.html
inspector/canvas/recording-webgl.html
js/dfg-arrayify-when-late-prevent-extensions.html
editing/selection/select-bidi-run.html
jquery/core.html
js/dom/dfg-ensure-non-array-array-storage-on-window.html

[EWS Watchlist]

Created attachment 340782 [details]
Archive of layout-test-results from ews114 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Keith Miller]

Created attachment 340893 [details]
Patch

[EWS Watchlist]

Attachment 340893 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/dfg/DFGArrayMode.h:519:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/JavaScriptCore/dfg/DFGArrayMode.h:520:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/JavaScriptCore/runtime/JSObject.h:827:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/JSObject.h:839:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/JSObject.h:849:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:57:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 6 in 77 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Keith Miller]

Created attachment 340894 [details]
Patch

[EWS Watchlist]

Attachment 340894 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/dfg/DFGArrayMode.h:519:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/JavaScriptCore/dfg/DFGArrayMode.h:520:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/JavaScriptCore/runtime/JSObject.h:827:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/JSObject.h:839:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/JSObject.h:849:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:57:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 6 in 78 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Keith Miller]

Created attachment 340900 [details]
Patch

[EWS Watchlist]

Attachment 340900 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/dfg/DFGArrayMode.h:519:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/JavaScriptCore/dfg/DFGArrayMode.h:520:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/JavaScriptCore/runtime/JSObject.h:827:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/JSObject.h:839:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/JSObject.h:849:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:57:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 6 in 78 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Comment on attachment 340900 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=340900&action=review

> Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:308
> +    static std::tuple<unsigned, IndexingType> decopressArrayAllocationProfile(UnlinkedArrayAllocationProfile compressedProfile)

*decompress, unless "decopress" is a term of art somehow

> Source/JavaScriptCore/dfg/DFGArrayMode.cpp:60
> +        if (action == Array::Write && observed & asArrayModes(toIndexingShape(type) | ArrayClass | CopyOnWrite))

Can you use some parentheses here?  a && b & c has unclear precedence.

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:874
> -    
> +

Revert

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:900
> -    
> +

Revert

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:2113
> -    
> +

Revert

> JSTests/ChangeLog:11
> [phone]  Keith Miller  <[e-mail]>
> +
> +        We should have a CoW storage for NewArrayBuffer arrays.
> +        https://bugs.webkit.org/show_bug.cgi?id=185003
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        * stress/put-on-cow-prototype.js: Added.
> +        (putByVal):
> +        (putById):
> +

Can has more tests? :-P

[Keith Miller]

Created attachment 340929 [details]
Patch

[EWS Watchlist]

Attachment 340929 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/dfg/DFGArrayMode.h:519:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/JavaScriptCore/dfg/DFGArrayMode.h:520:  Please declare enum bitfields as unsigned integral types.  [runtime/enum_bitfields] [5]
ERROR: Source/JavaScriptCore/runtime/JSObject.h:827:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/JSObject.h:839:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/JSObject.h:849:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:57:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 6 in 78 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 340929 [details]
Patch

Attachment 340929 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/7759552

New failing tests:
editing/selection/select-bidi-run.html

[EWS Watchlist]

Created attachment 340942 [details]
Archive of layout-test-results from ews112 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Keith Miller]

Created attachment 340989 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 340989 [details]
Patch for landing

Rejecting attachment 340989 [details] from commit-queue.

Number of test failures exceeded the failure limit.
Full output: http://webkit-queues.webkit.org/results/7766329

[WebKit Commit Bot]

Created attachment 340993 [details]
Archive of layout-test-results from webkit-cq-02 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: webkit-cq-02  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Keith Miller]

Committed r232070: <https://trac.webkit.org/changeset/232070>

[Yusuke Suzuki]

It seems that this change causes performance regression 38.3% in Kraken/crypto-aes, 12.6% in Kraken/crypto-ccm, and 3.6% in Kraken/desaturate. Can we fix this?

https://arewefastyet.com/#machine=29&view=breakdown&suite=kraken

[Yusuke Suzuki]

(In reply to Yusuke Suzuki from comment #30)
> It seems that this change causes performance regression 38.3% in
> Kraken/crypto-aes, 12.6% in Kraken/crypto-ccm, and 3.6% in
> Kraken/desaturate. Can we fix this?
> 
> https://arewefastyet.com/#machine=29&view=breakdown&suite=kraken

BTW, Octane/splay and Octane/splay-latency shows significant performance improvement.
https://arewefastyet.com/#machine=29&view=breakdown&suite=octane

[Keith Miller]

(In reply to Yusuke Suzuki from comment #30)
> It seems that this change causes performance regression 38.3% in
> Kraken/crypto-aes, 12.6% in Kraken/crypto-ccm, and 3.6% in
> Kraken/desaturate. Can we fix this?
> 
> https://arewefastyet.com/#machine=29&view=breakdown&suite=kraken

Interesting, I'll take a look. My first guess is that we somehow end up in an OSR loop.

(In reply to Yusuke Suzuki from comment #31)
> (In reply to Yusuke Suzuki from comment #30)
> > It seems that this change causes performance regression 38.3% in
> > Kraken/crypto-aes, 12.6% in Kraken/crypto-ccm, and 3.6% in
> > Kraken/desaturate. Can we fix this?
> > 
> > https://arewefastyet.com/#machine=29&view=breakdown&suite=kraken
> 
> BTW, Octane/splay and Octane/splay-latency shows significant performance
> improvement.
> https://arewefastyet.com/#machine=29&view=breakdown&suite=octane

Yay! It worked!

[Yusuke Suzuki]

https://arewefastyet.com/#machine=29&view=breakdown&suite=six-speed
Also, several microbenchmarks show performance regressions. I think they are nice for fixing issues since these microbenchmarks are small, easy to investigate.

[Keith Miller]

(In reply to Yusuke Suzuki from comment #33)
> https://arewefastyet.com/#machine=29&view=breakdown&suite=six-speed
> Also, several microbenchmarks show performance regressions. I think they are
> nice for fixing issues since these microbenchmarks are small, easy to
> investigate.

Ha, I just saw those as well!

[Yusuke Suzuki]

> (In reply to Yusuke Suzuki from comment #31)
> > (In reply to Yusuke Suzuki from comment #30)
> > > It seems that this change causes performance regression 38.3% in
> > > Kraken/crypto-aes, 12.6% in Kraken/crypto-ccm, and 3.6% in
> > > Kraken/desaturate. Can we fix this?
> > > 
> > > https://arewefastyet.com/#machine=29&view=breakdown&suite=kraken
> > 
> > BTW, Octane/splay and Octane/splay-latency shows significant performance
> > improvement.
> > https://arewefastyet.com/#machine=29&view=breakdown&suite=octane
> 
> Yay! It worked!

Yay!!!!!! :D

[Alexey Proskuryakov]

Looks like this broke a couple tests on 32-bit:

** The following JSC stress test failures have been introduced:
	ChakraCore.yaml/ChakraCore/test/es5/array_length.js.default
	stress/destructuring-rest-element.js.no-llint

[Ryan Haddad]

ChakraCore.yaml/ChakraCore/test/es5/array_length.js.default failing this assertion:

ASSERTION FAILED: m_isWritable
./runtime/Butterfly.h(89) : void JSC::ContiguousData<JSC::WriteBarrier<JSC::Unknown, DumbValueTraits<JSC::Unknown> > >::Data::clear() [T = JSC::WriteBarrier<JSC::Unknown, DumbValueTraits<JSC::Unknown> >]
1   0x10344a409 WTFCrash
2   0x1046a92e6 JSC::ContiguousData<JSC::WriteBarrier<JSC::Unknown, WTF::DumbValueTraits<JSC::Unknown> > >::Data::clear()
3   0x1046a70ac JSC::JSArray::setLength(JSC::ExecState*, unsigned int, bool)
4   0x1046a604b JSC::JSArray::defineOwnProperty(JSC::JSObject*, JSC::ExecState*, JSC::PropertyName, JSC::PropertyDescriptor const&, bool)
5   0x1047d41a1 JSC::objectConstructorDefineProperty(JSC::ExecState*)
6   0x79720d27177
7   0x1035394e2 llint_entry
8   0x79760d26c90
9   0x79720d2ce53
10  0x1035394e2 llint_entry
11  0x103530ed2 vmEntryToJavaScript
12  0x1043778ba JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*)
13  0x104376e63 JSC::Interpreter::executeProgram(JSC::SourceCode const&, JSC::ExecState*, JSC::JSObject*)
14  0x1046295a7 JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
15  0x102001149 runWithOptions(GlobalObject*, CommandLine&, bool&)
16  0x101fd8d6c jscmain(int, char**)::$_3::operator()(JSC::VM&, GlobalObject*, bool&) const
17  0x101fc0cb4 int runJSC<jscmain(int, char**)::$_3>(CommandLine, bool, jscmain(int, char**)::$_3 const&)
18  0x101fbf79f jscmain(int, char**)
19  0x101fbf6fe main
20  0x7fff6e830015 start

stress/destructuring-rest-element.js.no-llint is failing this one:
DFG ASSERTION FAILED: value.isType(typeFilterFor(variableAccessData->flushFormat()))
./dfg/DFGAbstractInterpreterInlines.h(280) : bool JSC::DFG::AbstractInterpreter<JSC::DFG::InPlaceAbstractState>::executeEffects(unsigned int, JSC::DFG::Node *) [AbstractStateType = JSC::DFG::InPlaceAbstractState]
1   0x2194ab WTFCrash
2   0x21ac94 WTFCrashWithInfo(int, char const*, char const*, int)
3   0x6c7acd JSC::DFG::AbstractInterpreter<JSC::DFG::InPlaceAbstractState>::executeEffects(unsigned int, JSC::DFG::Node*)
4   0x6c306d JSC::DFG::AbstractInterpreter<JSC::DFG::InPlaceAbstractState>::execute(unsigned int)
5   0x6c17c8 JSC::DFG::CFAPhase::performBlockCFA(JSC::DFG::BasicBlock*)
6   0x6c10ae JSC::DFG::CFAPhase::performForwardCFA()
7   0x6c0cff JSC::DFG::CFAPhase::run()
8   0x6c0488 bool JSC::DFG::runAndLog<JSC::DFG::CFAPhase>(JSC::DFG::CFAPhase&)
9   0x696ea2 bool JSC::DFG::runPhase<JSC::DFG::CFAPhase>(JSC::DFG::Graph&)
10  0x696e57 JSC::DFG::performCFA(JSC::DFG::Graph&)
11  0x9dcbe0 JSC::DFG::Plan::compileInThreadImpl()
12  0x9dae45 JSC::DFG::Plan::compileInThread(JSC::DFG::ThreadData*)
13  0x8073d8 JSC::DFG::compileImpl(JSC::VM&, JSC::CodeBlock*, JSC::CodeBlock*, JSC::DFG::CompilationMode, unsigned int, JSC::Operands<JSC::JSValue> const&, WTF::Ref<JSC::DeferredCompilationCallback, WTF::DumbPtrTraits<JSC::DeferredCompilationCallback> >&&)
14  0x806d99 JSC::DFG::compile(JSC::VM&, JSC::CodeBlock*, JSC::CodeBlock*, JSC::DFG::CompilationMode, unsigned int, JSC::Operands<JSC::JSValue> const&, WTF::Ref<JSC::DeferredCompilationCallback, WTF::DumbPtrTraits<JSC::DeferredCompilationCallback> >&&)
15  0xe13205 operationOptimize
16  0x40785c21
17  0x407864a4
18  0x4078345f
19  0x4277fc88
20  0x40781beb
21  0x30e160 vmEntryToJavaScript
22  0xda08b9 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*)
23  0xd9fd53 JSC::Interpreter::executeProgram(JSC::SourceCode const&, JSC::ExecState*, JSC::JSObject*)
24  0x10ab1a2 JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
25  0x7a610 runWithOptions(GlobalObject*, CommandLine&, bool&)
26  0x4b8ba jscmain(int, char**)::$_3::operator()(JSC::VM&, GlobalObject*, bool&) const
27  0x3150a int runJSC<jscmain(int, char**)::$_3>(CommandLine, bool, jscmain(int, char**)::$_3 const&)
28  0x2fca0 jscmain(int, char**)
29  0x2fbc7 main
30  0xa73f4611 start

[Saam Barati]

Comment on attachment 340989 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=340989&action=review

> Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h:312
> +        return std::make_tuple<unsigned, IndexingType>(WTFMove(profile), WTFMove(recommendedIndexingType));

No need for WTFMove here

> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:4583
> +            // TODO: Do I need this?

dunno

> Source/JavaScriptCore/runtime/CommonSlowPaths.h:255
> +inline JSArray* allocateNewArrayBuffer(VM& vm, Structure* structure, JSImmutableButterfly* immutableButterfly)

This name is misleading IMO. Should be something like allocateNewArrayWithImmutableButterfly or something like that

> Source/JavaScriptCore/runtime/JSObject.cpp:854
> +        if (!value.isInt32() || isCopyOnWrite(thisObject->indexingMode())) {

This isn't possible given above.

> Source/JavaScriptCore/runtime/JSObject.cpp:1487
> +    if (isCopyOnWrite(indexingMode()) && hasInt32(indexingMode())) {

This looks wrong.

[Saam Barati]

Comment on attachment 340989 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=340989&action=review

>> Source/JavaScriptCore/runtime/CommonSlowPaths.h:255
>> +inline JSArray* allocateNewArrayBuffer(VM& vm, Structure* structure, JSImmutableButterfly* immutableButterfly)
> 
> This name is misleading IMO. Should be something like allocateNewArrayWithImmutableButterfly or something like that

Ignore me. The name here is apt