Bug 73875

Summary: Add 8 bit paths to global object functions
Product: WebKit Reporter: Michael Saboff <msaboff>
Component: JavaScriptCoreAssignee: Michael Saboff <msaboff>
Status: RESOLVED FIXED    
Severity: Normal CC: ossy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 73950    
Bug Blocks: 73236    
Attachments:
Description Flags
Patch none

Description Michael Saboff 2011-12-05 16:15:57 PST
The functions decode(), parseInt(), parseFloat(), globalFuncEscape() and globalFuncUnescape() in JSGlobalObjectFunctions need 8 bit optimized paths.

These changes will benefit Kraken stanford-crypto-ccm and stanford-crypto-pbkdf2.
Comment 1 Michael Saboff 2011-12-05 17:37:43 PST
stanford-crypto-sha256-iterative also would benefit from this change.
Comment 2 Michael Saboff 2011-12-05 17:45:38 PST
Created attachment 117965 [details]
Patch

Results of proposed patch:

Generating benchmark report at Kraken_20111205_1735_benchReport.txt

Benchmark report for Kraken on msaboff-pro.apple.com (MacPro5,1).

VMs tested:
"Conf#1" at /Volumes/Data/src/webkit.baseline2/WebKitBuild/Release/jsc (r102016)
"Conf#2" at /Volumes/Data/src/webkit/WebKitBuild/Release/jsc (r102016)

Collected 12 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.

                                           Conf#1                  Conf#2                                     

ai-astar                             811.5160+-13.4857   ?   819.9420+-10.8933      ? might be 1.0104x slower
audio-beat-detection                 202.3366+-0.5953    !   203.8729+-0.6924       ! definitely 1.0076x slower
audio-dft                            259.9179+-2.1640    ?   260.1479+-2.4387       ?
audio-fft                            131.3159+-0.2710        131.1627+-0.1474       
audio-oscillator                     282.1622+-5.3042        277.9279+-5.2296         might be 1.0152x faster
imaging-darkroom                     337.4726+-6.2865        331.4176+-4.3442         might be 1.0183x faster
imaging-desaturate                   239.1415+-0.2812    ^   238.7468+-0.1038       ^ definitely 1.0017x faster
imaging-gaussian-blur                609.6771+-0.3478    ?   610.2160+-0.3388       ?
json-parse-financial                  71.8287+-0.2332         71.6736+-0.3560       
json-stringify-tinderbox              81.8360+-0.1419    ?    81.8690+-0.1723       ?
stanford-crypto-aes                  118.8075+-0.7028    ?   118.8253+-0.4006       ?
stanford-crypto-ccm                  118.0467+-0.7520    ^   116.1189+-0.9503       ^ definitely 1.0166x faster
stanford-crypto-pbkdf2               232.7204+-1.0608    ?   233.9356+-1.3221       ?
stanford-crypto-sha256-iterative      96.7228+-0.1396    ^    94.3186+-0.3950       ^ definitely 1.0255x faster

<arithmetic> *                       256.6787+-1.0148        256.4411+-0.9486       
<geometric>                          198.1845+-0.4079        197.4195+-0.5044       
<harmonic>                           159.7440+-0.2318    ^   158.8841+-0.3157       ^ definitely 1.0054x faster

No change for stanford-crypto-pbkdf2 even though it uses the tuned code paths.
Comment 3 Michael Saboff 2011-12-06 09:21:25 PST
Committed r102146: <http://trac.webkit.org/changeset/102146>
Comment 4 Csaba Osztrogonác 2011-12-06 11:38:08 PST
(In reply to comment #3)
> Committed r102146: <http://trac.webkit.org/changeset/102146>

It broke fast/js/encode-URI-test.html on SL, GTK, Qt:

--- /Volumes/Data/slave/snowleopard-intel-release-tests/build/layout-test-results/fast/js/encode-URI-test-expected.txt 
+++ /Volumes/Data/slave/snowleopard-intel-release-tests/build/layout-test-results/fast/js/encode-URI-test-actual.txt 
@@ -1,1 +1,3 @@
-Testing complete. No failures.
+called unescape on "%uxxxx" and got "uxxxx" instead of "%uxxxx"
+
+Testing complete. 1 tests failed.

Could you check it, please?
Comment 5 Csaba Osztrogonác 2011-12-06 11:38:27 PST
Comment on attachment 117965 [details]
Patch

remove r+ from landed patch
Comment 6 Michael Saboff 2011-12-06 11:41:00 PST
(In reply to comment #4)
> (In reply to comment #3)
> > Committed r102146: <http://trac.webkit.org/changeset/102146>
> 
> It broke fast/js/encode-URI-test.html on SL, GTK, Qt:
> 
> --- /Volumes/Data/slave/snowleopard-intel-release-tests/build/layout-test-results/fast/js/encode-URI-test-expected.txt 
> +++ /Volumes/Data/slave/snowleopard-intel-release-tests/build/layout-test-results/fast/js/encode-URI-test-actual.txt 
> @@ -1,1 +1,3 @@
> -Testing complete. No failures.
> +called unescape on "%uxxxx" and got "uxxxx" instead of "%uxxxx"
> +
> +Testing complete. 1 tests failed.
> 
> Could you check it, please?

I'll look into this.
Comment 7 Michael Saboff 2011-12-06 14:02:19 PST
(In reply to comment #4)
> (In reply to comment #3)
> > Committed r102146: <http://trac.webkit.org/changeset/102146>
> 
> It broke fast/js/encode-URI-test.html on SL, GTK, Qt:
> 
> --- /Volumes/Data/slave/snowleopard-intel-release-tests/build/layout-test-results/fast/js/encode-URI-test-expected.txt 
> +++ /Volumes/Data/slave/snowleopard-intel-release-tests/build/layout-test-results/fast/js/encode-URI-test-actual.txt 
> @@ -1,1 +1,3 @@
> -Testing complete. No failures.
> +called unescape on "%uxxxx" and got "uxxxx" instead of "%uxxxx"
> +
> +Testing complete. 1 tests failed.
> 
> Could you check it, please?

Tracked in https://bugs.webkit.org/show_bug.cgi?id=73950
Comment 8 Michael Saboff 2011-12-06 14:43:56 PST
Closed due to the closing of the related bug https://bugs.webkit.org/show_bug.cgi?id=73950 that tracked the test failure.