Summary: | [JSC] Optimize Object.prototype.toString | ||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yusuke Suzuki <ysuzuki> | ||||||||||||||||||||||||||
Component: | New Bugs | Assignee: | Yusuke Suzuki <ysuzuki> | ||||||||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||||||||
Severity: | Normal | CC: | ews-watchlist, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer | ||||||||||||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||||||||
Bug Depends on: | 193037 | ||||||||||||||||||||||||||||
Bug Blocks: | |||||||||||||||||||||||||||||
Attachments: |
|
Description
Yusuke Suzuki
2018-12-24 23:46:22 PST
Created attachment 358056 [details]
Patch
Created attachment 358057 [details]
Patch
Created attachment 358058 [details]
Patch
Created attachment 358060 [details]
Patch
Comment on attachment 358060 [details] Patch Attachment 358060 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/10541563 New failing tests: media/modern-media-controls/airplay-support/airplay-support-disable-event-listeners-until-play.html Created attachment 358062 [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 358078 [details]
Patch
Comment on attachment 358078 [details] Patch Attachment 358078 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/10551775 New failing tests: media/modern-media-controls/airplay-support/airplay-support-disable-event-listeners-until-play.html Created attachment 358080 [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 358110 [details]
Patch
Created attachment 358111 [details]
Patch
Comment on attachment 358111 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=358111&action=review > Source/JavaScriptCore/ChangeLog:9 > + It is called so many times in wtb-lebab.js. This patch optimizes Did you mean to say more in this sentence? If not, I would get rid of the "so" and make this: "It is called many times in wtb-lebab.js, for instance" > Source/JavaScriptCore/ChangeLog:12 > + 1. We should emit code looking up cached one in DFG and FTL. Nit: cached one => a cached result. > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:12473 > + auto isUndefined = m_jit.branchIfUndefined(sourceRegs); > + m_jit.move(TrustedImmPtr::weakPointer(m_jit.graph(), m_jit.vm()->smallStrings.nullObjectString()), resultGPR); > + auto done = m_jit.jump(); > + isUndefined.link(&m_jit); > + m_jit.move(TrustedImmPtr::weakPointer(m_jit.graph(), m_jit.vm()->smallStrings.undefinedObjectString()), resultGPR); > + done.link(&m_jit); Don't you need a case here for booleans? If so, can you add a test? > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:6449 > + LBasicBlock undefinedCase = m_out.newBlock(); > + LBasicBlock nullCase = m_out.newBlock(); > + LBasicBlock continuation = m_out.newBlock(); > + > + speculate(m_node->child1()); > + LValue source = lowJSValue(m_node->child1(), ManualOperandSpeculation); > + LValue valueIsUndefined = m_out.equal(source, m_out.constInt64(ValueUndefined)); > + m_out.branch(valueIsUndefined, unsure(undefinedCase), unsure(nullCase)); > + > + LBasicBlock lastNext = m_out.appendTo(undefinedCase, nullCase); > + ValueFromBlock undefinedString = m_out.anchor(weakPointer(vm().smallStrings.undefinedObjectString())); > + m_out.jump(continuation); > + > + m_out.appendTo(nullCase, continuation); > + ValueFromBlock nullString = m_out.anchor(weakPointer(vm().smallStrings.nullObjectString())); > + m_out.jump(continuation); > + > + m_out.appendTo(continuation, lastNext); > + setJSValue(m_out.phi(pointerType(), undefinedString, nullString)); > + return; Ditto on the boolean here. Comment on attachment 358111 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=358111&action=review >> Source/JavaScriptCore/ChangeLog:9 >> + It is called so many times in wtb-lebab.js. This patch optimizes > > Did you mean to say more in this sentence? If not, I would get rid of the "so" and make this: "It is called many times in wtb-lebab.js, for instance" Yeah, fixed by dropping `so`. >> Source/JavaScriptCore/ChangeLog:12 >> + 1. We should emit code looking up cached one in DFG and FTL. > > Nit: cached one => a cached result. Nice. Fixed. >> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:12473 >> + done.link(&m_jit); > > Don't you need a case here for booleans? If so, can you add a test? Oops, fixed to use OtherUse and SpecOther. Misc is not right. >> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:6449 >> + return; > > Ditto on the boolean here. Fixed. Created attachment 358166 [details]
Patch
Created attachment 358211 [details]
Patch
Created attachment 358214 [details]
Patch
Ping? Comment on attachment 358214 [details]
Patch
r=me
Comment on attachment 358214 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=358214&action=review > Source/JavaScriptCore/ChangeLog:12 > + 1. We should emit code looking up cached one in DFG and FTL. cached one => cached to string Comment on attachment 358214 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=358214&action=review Thank you! >> Source/JavaScriptCore/ChangeLog:12 >> + 1. We should emit code looking up cached one in DFG and FTL. > > cached one => cached to string Oops, fixed. Committed r239612: <https://trac.webkit.org/changeset/239612> |