| Summary: | We should have a CoW storage for NewArrayBuffer arrays. | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Keith Miller <keith_miller> | ||||||||||||||||||||||||||||||
| Component: | JavaScriptCore | Assignee: | Keith Miller <keith_miller> | ||||||||||||||||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||||||||||||||||
| Severity: | Normal | CC: | ap, commit-queue, ews-watchlist, fpizlo, mark.lam, msaboff, realdawei, rniwa, ryanhaddad, saam, webkit-bug-importer, ysuzuki | ||||||||||||||||||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||||||||||||||||||
| OS: | Unspecified | ||||||||||||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||||||||||||
|
Description
Keith Miller
2018-04-25 15:25:43 PDT
Created attachment 340710 [details]
Patch
Created attachment 340771 [details]
Patch
Created attachment 340776 [details]
Patch
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.
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 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
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 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
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 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 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
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 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
Created attachment 340893 [details]
Patch
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.
Created attachment 340894 [details]
Patch
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.
Created attachment 340900 [details]
Patch
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.
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 > +2018-05-21 Keith Miller <keith_miller@apple.com> > + > + 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 Created attachment 340929 [details]
Patch
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.
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 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
Created attachment 340989 [details]
Patch for landing
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 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
Committed r232070: <https://trac.webkit.org/changeset/232070> 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 (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 (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! 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. (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! > (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
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 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 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. 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 |