Bug 76861 – JSValue::toString() should return a JSString* instead of a UString

• Component: New Bugs
• Status: RESOLVED FIXED

[Geoffrey Garen]

JSValue::toString() should return a JSString* instead of a UString

[Geoffrey Garen]

Created attachment 123624 [details]
Patch

[Geoffrey Garen]

Seems performance-neutral:

Benchmark report for SunSpider, V8, and Kraken on garen (MacPro5,1).

VMs tested:
"BASELINE" at /Volumes/Big/ggaren/baseline-webkit/WebKitBuild/Release/jsc (r104476)
"PATCH" at /Volumes/Big/ggaren/webkit/WebKitBuild/Release/jsc (r105540)

Collected 24 samples per benchmark/VM, with 4 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                 PATCH                                      
SunSpider:
   3d-cube                                6.6110+-0.0354    ?     6.6801+-0.0353       ? might be 1.0104x slower
   3d-morph                              10.3602+-0.0463         10.3560+-0.0460       
   3d-raytrace                            8.7381+-0.0234    !     8.8391+-0.0526       ! definitely 1.0116x slower
   access-binary-trees                    1.8818+-0.0097    ?     1.8820+-0.0115       ?
   access-fannkuch                        9.0918+-0.0154    !     9.4960+-0.0081       ! definitely 1.0445x slower
   access-nbody                           4.6114+-0.0194          4.5918+-0.0121       
   access-nsieve                          4.0541+-0.0623          3.9881+-0.0236         might be 1.0165x faster
   bitops-3bit-bits-in-byte               1.4689+-0.0064          1.4661+-0.0016       
   bitops-bits-in-byte                    6.0210+-0.0231    ?     6.0327+-0.0146       ?
   bitops-bitwise-and                     3.8085+-0.0220          3.7922+-0.0066       
   bitops-nsieve-bits                     6.5441+-0.0275    ?     6.5810+-0.0313       ?
   controlflow-recursive                  2.7080+-0.0083          2.7039+-0.0075       
   crypto-aes                             8.3035+-0.0603          8.2565+-0.0427       
   crypto-md5                             2.9664+-0.0225          2.9416+-0.0217       
   crypto-sha1                            2.6637+-0.0277          2.6326+-0.0139         might be 1.0118x faster
   date-format-tofte                     11.8221+-0.0504         11.7626+-0.0614       
   date-format-xparb                     11.0695+-0.1334    !    11.4676+-0.0735       ! definitely 1.0360x slower
   math-cordic                            8.3628+-0.0210    ?     8.4903+-0.1125       ? might be 1.0153x slower
   math-partial-sums                     12.0228+-0.0349         12.0026+-0.0230       
   math-spectral-norm                     3.0567+-0.0046          3.0542+-0.0032       
   regexp-dna                            10.0438+-0.0567         10.0245+-0.0550       
   string-base64                          5.0494+-0.0270    !     5.1369+-0.0312       ! definitely 1.0173x slower
   string-fasta                           8.3458+-0.0318    ^     8.1463+-0.0230       ^ definitely 1.0245x faster
   string-tagcloud                       14.6002+-0.0521    ^    14.4003+-0.0540       ^ definitely 1.0139x faster
   string-unpack-code                    23.3418+-0.0767    ?    23.4400+-0.0713       ?
   string-validate-input                  6.7119+-0.0397          6.6901+-0.0234       

   <arithmetic> *                         7.4715+-0.0170    ?     7.4944+-0.0155       ? might be 1.0031x slower
   <geometric>                            6.1247+-0.0122    ?     6.1343+-0.0118       ? might be 1.0016x slower
   <harmonic>                             4.8916+-0.0094          4.8883+-0.0098         might be 1.0007x faster

                                             BASELINE                 PATCH                                      
V8:
   crypto                                89.5955+-0.1737         89.5781+-0.1318       
   deltablue                            194.0910+-1.7153        192.3804+-1.3365       
   earley-boyer                         108.7625+-1.5667        108.4443+-1.6310       
   raytrace                              60.7802+-0.2804         60.5074+-0.2504       
   regexp                               111.5169+-0.2266    ?   111.6835+-0.2607       ?
   richards                             156.0449+-0.1767    ?   156.2165+-0.1462       ?
   splay                                 85.0744+-1.2860         83.9436+-0.2382         might be 1.0135x faster

   <arithmetic>                         115.1236+-0.5042        114.6791+-0.3207         might be 1.0039x faster
   <geometric> *                        107.8749+-0.4574        107.4618+-0.2753         might be 1.0038x faster
   <harmonic>                           101.2839+-0.4243        100.8785+-0.2366         might be 1.0040x faster

                                             BASELINE                 PATCH                                      
Kraken:
   ai-astar                             947.2241+-0.8833        946.5622+-0.8812       
   audio-beat-detection                 222.4434+-0.4525    ?   222.5369+-0.4888       ?
   audio-dft                            326.8621+-3.3635        324.7562+-0.3348       
   audio-fft                            138.5094+-0.0789        138.5020+-0.1028       
   audio-oscillator                     352.7950+-2.6635        350.4292+-1.9507       
   imaging-darkroom                     338.9531+-4.8759        337.4513+-5.7336       
   imaging-desaturate                   265.1648+-0.0966    ?   265.6183+-0.3809       ?
   imaging-gaussian-blur                604.0797+-3.2211        601.5251+-2.0723       
   json-parse-financial                  78.4658+-0.1491    ^    78.0021+-0.1352       ^ definitely 1.0059x faster
   json-stringify-tinderbox              93.6828+-0.3702    ^    92.8008+-0.2356       ^ definitely 1.0095x faster
   stanford-crypto-aes                  120.4857+-0.2230    !   121.7085+-0.3545       ! definitely 1.0101x slower
   stanford-crypto-ccm                  119.6886+-1.0952        118.8119+-0.8683       
   stanford-crypto-pbkdf2               228.5304+-0.8767    ^   226.2212+-0.4595       ^ definitely 1.0102x faster
   stanford-crypto-sha256-iterative     103.8256+-0.0686    !   104.7056+-0.2849       ! definitely 1.0085x slower

   <arithmetic> *                       281.4793+-0.5644        280.6880+-0.4083         might be 1.0028x faster
   <geometric>                          215.2715+-0.3574        214.7396+-0.2655         might be 1.0025x faster
   <harmonic>                           172.5559+-0.2388        172.1969+-0.1845         might be 1.0021x faster

                                             BASELINE                 PATCH                                      
All benchmarks:
   <arithmetic>                         105.1242+-0.2135        104.8349+-0.1529         might be 1.0028x faster
   <geometric>                           27.1091+-0.0520         27.0970+-0.0454         might be 1.0004x faster
   <harmonic>                             8.5994+-0.0164          8.5931+-0.0170         might be 1.0007x faster

                                             BASELINE                 PATCH                                      
Geomean of preferred means:
   <scaled-result>                       60.9894+-0.1397         60.9169+-0.1087         might be 1.0012x faster

[Gavin Barraclough]

Comment on attachment 123624 [details]
Patch

->value(exec)->value(exec)->value(exec)->value(exec)->value(exec)->value(exec)->value(exec)->value(exec)->value(exec)->value(exec)->value(exec)->value(exec)->value(exec)->value(exec)->value(exec)->value(exec)->value(exec)->value(exec)->value(exec)->value(exec)->value(exec)->value(exec)->value(exec).

[Early Warning System Bot]

Comment on attachment 123624 [details]
Patch

Attachment 123624 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/11117534

[Geoffrey Garen]

Created attachment 123634 [details]
Patch

[Geoffrey Garen]

Created attachment 123644 [details]
Patch

[Geoffrey Garen]

Created attachment 123649 [details]
Patch

[Geoffrey Garen]

Created attachment 123662 [details]
Patch

[Geoffrey Garen]

Created attachment 123671 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 123671 [details]
Patch

Attachment 123671 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/11117611

[Geoffrey Garen]

Created attachment 123695 [details]
Patch

[Geoffrey Garen]

Committed r105698: <http://trac.webkit.org/changeset/105698>

[Csaba Osztrogonác]

Reopen, because it broke 2 tests at least on Qt:

--- /ramdisk/qt-linux-64-release-quicktest/build/layout-test-results/fast/js/array-functions-non-arrays-expected.txt 
+++ /ramdisk/qt-linux-64-release-quicktest/build/layout-test-results/fast/js/array-functions-non-arrays-actual.txt 
@@ -57,9 +57,9 @@
 PASS Array.prototype.slice.call(new TwoItemConstructor, 0, 1) is ['b']
 PASS properties(Array.prototype.sort.call({})) is ''
 PASS properties(Array.prototype.sort.call(['b', 'a'])) is '0:a, 1:b, length:2(DontDelete, DontEnum)'
-PASS properties(Array.prototype.sort.call({ length:2, 0:'b', 1:'a' })) is '0:a, 1:b, length:2'
+FAIL properties(Array.prototype.sort.call({ length:2, 0:'b', 1:'a' })) should be 0:a, 1:b, length:2. Was 0:b, 1:a, length:2.
 PASS properties(Array.prototype.sort.call(new OneItemConstructor)) is '0:a(FromPrototype), length:1(FromPrototype)'
-PASS properties(Array.prototype.sort.call(new TwoItemConstructor)) is '0:a, 1:b, length:2(FromPrototype)'
+FAIL properties(Array.prototype.sort.call(new TwoItemConstructor)) should be 0:a, 1:b, length:2(FromPrototype). Was 0:b(FromPrototype), 1:a(FromPrototype), length:2(FromPrototype).
 PASS Array.prototype.splice.call({}, 0, 1) is []
 PASS Array.prototype.splice.call(['b', 'a'], 0, 1) is ['b']
 PASS Array.prototype.splice.call({ length:2, 0:'b', 1:'a' }, 0, 1) is ['b']


--- /ramdisk/qt-linux-64-release-quicktest/build/layout-test-results/sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.11_Array_prototype_sort/S15.4.4.11_A3_T1-expected.txt 
+++ /ramdisk/qt-linux-64-release-quicktest/build/layout-test-results/sputnik/Conformance/15_Native_Objects/15.4_Array/15.4.4/15.4.4.11_Array_prototype_sort/S15.4.4.11_A3_T1-actual.txt 
@@ -1,6 +1,6 @@
 S15.4.4.11_A3_T1
 
-PASS 
+FAIL SputnikError: #1: Check ToString operator
 
 TEST COMPLETE

[Csaba Osztrogonác]

These tests fail on SL too.

And this patch broke GTK build too:
../../Source/WebCore/bindings/js/JSPeerConnectionCustom.cpp: In static member function ‘static void* WebCore::JSPeerConnectionConstructor::constructJSPeerConnection(JSC::ExecState*)’:
../../Source/WebCore/bindings/js/JSPeerConnectionCustom.cpp:51: error: invalid initialization of reference of type ‘const JSC::UString&’ from expression of type ‘JSC::JSString*’
../../Source/WebCore/bindings/js/JSDOMBinding.h:322: error: in passing argument 1 of ‘WTF::String WebCore::ustringToString(const JSC::UString&)’

And an interpreter buildfix landed in http://trac.webkit.org/changeset/105702

[Ilya Tikhonovsky]

build fixes for Qt and Gtk:

http://trac.webkit.org/changeset/105713
http://trac.webkit.org/changeset/105703
http://trac.webkit.org/changeset/105702

please check that changes are fine.

[Csaba Osztrogonác]

I skipped these tests on Qt: http://trac.webkit.org/changeset/105715
Please unskip them with the proper fix.

[Geoffrey Garen]

> build fixes for Qt and Gtk:

These are fine. Thanks!

[Geoffrey Garen]

Created attachment 123789 [details]
Patch

[WebKit Review Bot]

Comment on attachment 123789 [details]
Patch

Attachment 123789 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11343056

New failing tests:
media/audio-garbage-collect.html

[Geoffrey Garen]

Committed r105811: <http://trac.webkit.org/changeset/105811>

[Geoffrey Garen]

Bindings test expectations fixed:

Committed r105813: <http://trac.webkit.org/changeset/105813>

[Csaba Osztrogonác]

I unskipped the now passing tests - http://trac.webkit.org/changeset/105853