Refactor HTMLTokenizer::write to make it more readable and split out the lexer bits from flow control bits
Created attachment 56412 [details] Patch
Attachment 56412 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 WebCore/html/HTMLTokenizer.cpp:1688: One line control clauses should not use braces. [whitespace/braces] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 56412 [details] Patch r- for lack of performance testing.
Please advise how you would like me to performance test.
Comment on attachment 56412 [details] Patch I used run-pageloadtest cvs-base (as provided by Maciej), edited to run the test 10 times each (instead of 3). I discarded the first 2 runs for each (as they seemed to be noisy). % grep FINISHED without_patch.txt | tail -8 | awk '{s += $5; print $5} END { print "avg: ", s/NR }' 2169.3 2179.2 2173.5 2157.4 2191.8 2179.0 2157.5 2137.7 avg: 2168.18 % grep FINISHED with_patch2.txt | tail -8 | awk '{s += $5; print $5} END { print "avg: ", s/NR }' 2193.4 2126.9 2226.7 2207.0 2177.4 2191.7 2186.0 2177.1 avg: 2185.77 2185.77 - 2168.18 = 17.59 17.59 / 2168.18 = 0.00811279506 Is that convincing?
Adding "inline" to the newly split function makes 100 plt runs claim this is a 0.5% progression. I think it's noise, but at least the data doesn't seem to think it's a regression.
Created attachment 56442 [details] Marked advance() as inline
Attachment 56442 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 WebCore/html/HTMLTokenizer.cpp:1688: One line control clauses should not use braces. [whitespace/braces] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 56443 [details] results from 100 plt runs w/o the patch
Created attachment 56444 [details] results from 100 plt runs w/ the inline patch
I think I give up. After an afternoon of fighting our broken perf-tests, I can't prove this is better or worse. I think it's a wash, but I can't prove it.
Ahha! Wrote a new benchmark. :) The new benchmark (soon to be attached to bug 39338) correctly shows that the orginal patch and the "inline" patch are both regressions. But using ALWAYS_INLINE on the new function shows no change. r59687 Release Build: Running 20 times Ignoring warm-up run (4184) 3700 3741 3784 3735 3726 3769 3823 3775 3770 3800 3888 3811 3859 3920 3864 3950 3997 3921 3987 4027 avg 3842.35 stdev 95.62231695582365 r59687 "inline" patch applied: Running 20 times Ignoring warm-up run (4447) 4000 4132 3981 4047 4048 4055 4130 4021 4105 4071 4120 4161 4133 4215 4127 4204 4148 4228 4171 4256 avg 4117.65 stdev 75.01484853014102 r59687 with ALWAYS_INLINE patch applied: Running 20 times Ignoring warm-up run (4130) 3690 3728 3776 3759 3732 3782 3843 3787 3786 3841 3910 3826 3878 3960 3876 3932 4020 3937 4015 4001 avg 3853.95 stdev 97.88435779020057
Created attachment 56461 [details] Patch
Attachment 56461 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 WebCore/html/HTMLTokenizer.cpp:1688: One line control clauses should not use braces. [whitespace/braces] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 56461 [details] Patch r=me
Committed r59749: <http://trac.webkit.org/changeset/59749>