Bug 156680

Summary: [INTL] Use @thisNumberValue instead of `instanceof @Number`
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: JavaScriptCoreAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, commit-queue, keith_miller, mark.lam, msaboff, rniwa, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews101 for mac-yosemite
none
Archive of layout-test-results from ews104 for mac-yosemite-wk2
none
Archive of layout-test-results from ews125 for ios-simulator-wk2
none
Archive of layout-test-results from ews114 for mac-yosemite
none
Patch
saam: review+
Patch for landing none

Description Yusuke Suzuki 2016-04-17 14:39:45 PDT
Use @thisNumberValue instead of `instanceof @Number`.
`instanceof @Number` is not enough;
For example, given 2 realms, the object created in one realm does not inherit the Number of another realm.
Another example is that the object which does not inherit Number.

```
var number = new Number(42);
number.__proto__ = null;
```
Comment 1 Yusuke Suzuki 2016-04-17 16:56:46 PDT
Created attachment 276611 [details]
Patch
Comment 2 Build Bot 2016-04-17 17:54:20 PDT
Comment on attachment 276611 [details]
Patch

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

New failing tests:
js/number-toLocaleString.html
Comment 3 Build Bot 2016-04-17 17:54:22 PDT
Created attachment 276615 [details]
Archive of layout-test-results from ews101 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 4 Build Bot 2016-04-17 17:54:50 PDT
Comment on attachment 276611 [details]
Patch

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

New failing tests:
js/number-toLocaleString.html
Comment 5 Build Bot 2016-04-17 17:54:53 PDT
Created attachment 276616 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 6 Build Bot 2016-04-17 17:59:51 PDT
Comment on attachment 276611 [details]
Patch

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

New failing tests:
js/number-toLocaleString.html
Comment 7 Build Bot 2016-04-17 17:59:54 PDT
Created attachment 276617 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.10.5
Comment 8 Build Bot 2016-04-17 18:08:33 PDT
Comment on attachment 276611 [details]
Patch

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

New failing tests:
js/number-toLocaleString.html
Comment 9 Build Bot 2016-04-17 18:08:36 PDT
Created attachment 276618 [details]
Archive of layout-test-results from ews114 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 10 Yusuke Suzuki 2016-04-18 10:53:35 PDT
Created attachment 276647 [details]
Patch
Comment 11 Saam Barati 2016-04-18 12:39:06 PDT
Comment on attachment 276647 [details]
Patch

r=me with build fix
Comment 12 Yusuke Suzuki 2016-04-18 15:13:11 PDT
Created attachment 276674 [details]
Patch for landing
Comment 13 Yusuke Suzuki 2016-04-19 06:33:50 PDT
Committed r199725: <http://trac.webkit.org/changeset/199725>
Comment 14 Yusuke Suzuki 2016-04-25 01:21:24 PDT
date-format-xparb regression occurs during r199725 - r199739.
While I'm not convinced that this patch is the cause of the above regression yet, anyway, worth investigating. (I'll do it)

http://trac.webkit.org/log/?action=stop_on_copy&mode=stop_on_copy&rev=199739&stop_rev=199725&limit=100
Comment 15 Yusuke Suzuki 2016-04-25 01:39:36 PDT
I've just measured and found that this patch is unrelated.
Comment 16 Yusuke Suzuki 2016-04-25 01:47:30 PDT
Hm, while the figure shows the significant regression[1], I cannot see any regression between r199724 - r199739. strange.

Warning: could not identify checkout location for baseline
Warning: could not identify checkout location for patched
./runscript: 5: ./runscript: /usr/sbin/sysctl: not found
112/112                                          
Generating benchmark report at /home/yusukesuzuki/dev/WebKit/baseline_patched_SunSpider_gochiusa_20160425_1745_report.txt
And raw data at /home/yusukesuzuki/dev/WebKit/baseline_patched_SunSpider_gochiusa_20160425_1745.json

Benchmark report for SunSpider on gochiusa.

VMs tested:
"baseline" at /home/yusukesuzuki/dev/WebKit/WebKitBuild/r199724/Release/bin/jsc
"patched" at /home/yusukesuzuki/dev/WebKit/WebKitBuild/r199739/Release/bin/jsc

Collected 30 samples per benchmark/VM, with 30 VM invocations per benchmark. Emitted a call to gc()
between sample measurements. Used 1 benchmark iteration per VM invocation for warm-up. Used the
jsc-specific preciseTime() function to get microsecond-level timing. Reporting benchmark execution
times with 95% confidence intervals in milliseconds.

                            baseline                  patched                                      

date-format-xparb        6.5764+-0.2597            6.5555+-0.2789        

<arithmetic>             6.5764+-0.2597            6.5555+-0.2789          might be 1.0032x faster



[1]: https://arewefastyet.com/#machine=29&view=single&suite=ss&subtest=format-xparb