Bug 185065

Summary: [JSC] Add SameValue DFG node
Product: WebKit Reporter: goktug
Component: JavaScriptCoreAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, mark.lam, msaboff, ryanhaddad, saam, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: Safari 11   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=173226
Attachments:
Description Flags
Patch
none
Patch
none
Patch saam: review+

goktug
Reported 2018-04-26 20:04:20 PDT
It seems like Object.is has very poor performance; even polyfill performs much better than the built-in method (2x). Also for comparison "===" performs ~10x. See the following benchmarks: https://jsperf.com/string-object-is-performance https://jsperf.com/object-is-vs-polyfill You can see similar issue fixed for V8 here: https://bugs.chromium.org/p/v8/issues/detail?id=7007
Attachments
Patch (42.37 KB, patch)
2018-04-28 16:35 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (47.37 KB, patch)
2018-04-28 18:52 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (47.37 KB, patch)
2018-04-28 18:54 PDT, Yusuke Suzuki 		saam: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2018-04-26 20:37:39 PDT
<rdar://problem/39781064>
Yusuke Suzuki
Comment 2 2018-04-28 16:35:38 PDT
Created attachment 339081 [details] Patch WIP: 64bit works.
Yusuke Suzuki
Comment 3 2018-04-28 18:52:44 PDT
Created attachment 339084 [details] Patch
Yusuke Suzuki
Comment 4 2018-04-28 18:54:12 PDT
Created attachment 339085 [details] Patch
Yusuke Suzuki
Comment 5 2018-05-01 20:30:17 PDT
Ping?
Saam Barati
Comment 6 2018-05-01 21:53:35 PDT
Comment on attachment 339085 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=339085&action=review > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2643 > + if (argumentCountIncludingThis != 3) Why not < instead of !=? > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:6383 > + m_jit.compareDouble(CCallHelpers::DoubleNotEqualOrUnordered, arg1FPR, arg1FPR, tempGPR); Since this code branches internally, it feels like we should only do it if the doubles aren’t bitwise equal
Yusuke Suzuki
Comment 7 2018-05-01 22:49:44 PDT
Comment on attachment 339085 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=339085&action=review >> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2643 >> + if (argumentCountIncludingThis != 3) > > Why not < instead of !=? Fixed. >> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:6383 >> + m_jit.compareDouble(CCallHelpers::DoubleNotEqualOrUnordered, arg1FPR, arg1FPR, tempGPR); > > Since this code branches internally, it feels like we should only do it if the doubles aren’t bitwise equal OK, fixed.
Yusuke Suzuki
Comment 8 2018-05-01 22:51:39 PDT
Committed r231224: <https://trac.webkit.org/changeset/231224>
Ryan Haddad
Comment 9 2018-05-02 09:16:58 PDT
The test added with this change is failing on the 32-bit JSC bot: stress/object-is.js.default: Exception: Error: bad value: true stress/object-is.js.default: shouldBe@object-is.js:3:24 stress/object-is.js.default: global code@object-is.js:67:13 stress/object-is.js.default: ERROR: Unexpected exit code: 3 FAIL: stress/object-is.js.default https://build.webkit.org/builders/Apple%20High%20Sierra%2032-bit%20JSC%20%28BuildAndTest%29/builds/1808
Yusuke Suzuki
Comment 10 2018-05-02 17:42:17 PDT
Committed r231284: <https://trac.webkit.org/changeset/231284>
Note You need to log in before you can comment on or make changes to this bug.