A new HTML 5 parser landed in WebKit in r61234: http://trac.webkit.org/changeset/61234 This new parser was turned on in Chromium in http://src.chromium.org/viewvc/chrome?view=rev&revision=49936 Ever since that change, the Vista and OSX 10.6 perf bots started failing. The Vista bot consistently fails, while the OSX 10.6 one is flaky: http://build.chromium.org/buildbot/waterfall/builders/Mac10.6%20Perf(1)/builds/4306 http://build.chromium.org/buildbot/waterfall/builders/Vista%20Perf%20(1)/builds/5397 See also the first regression in the following graph: http://build.chromium.org/buildbot/perf/vista-release-dual-core/intl1/report.html?history=150&rev=-1
Adding some more data to the bug. First, this only seems to happen on the Vista cycler (XP is fine). Second, the regression seems to be due to a few outliers, rather than a consistent move across all runs (see below). This is a good thing as it means it is probably a bug rather than something inherently slower about the new parser. Sample run times: [68,65,149,41,49,60,14,183,49,139,104,114,230,140,90,224,84,70,191,38,65,69,102,209,77,53,148,137,95,28,43365,39,61398,63,129,170,98,131,38,168,85,43696,84,203,220,143,93,102,403,199,258,306,210,294,122,312,27,10,74,27,24,47,11,111,28,89,69,67,92,64,56,122,72,49,119,30,40,43,69,129,41,34,88,98,76,18,108,48,215,50,76,115,76,78,31,80,47,21389,64,149,168,115,72,60,280,140,216,214,168,214,90,231,28,10,74,28,25,44,10,112,30,90,70,68,96,63,58,123,71,45,119,28,39,40,72,132,41,34,83,97,76,18,108,48,214,52,76,118,75,75,30,79,47,79,65,147,167,117,61,60,278,137,221,203,157,206,89,227,28,11,74,28,25,43,9,112,28,90,69,67,92,64,58,122,69,45,121,28,38,40,70,131,41,34,83,122,77,17,109,48,215,51,77,117,76,77,31,79,47,79,64,148,166,114,61,60,279,135,218,199,175,206,88,229,27,10,73,28,25,44,10,111,29,90,69,70,92,66,58,125,69,45,118,28,38,40,72,130,41,35,83,121,74,18,111,29,216,52,76,116,74,76,31,81,47,79,64,148,167,114,62,60,282,139,231,205,165,206,89,229,28,10,74,28,26,44,10,113,29,90,70,68,96,65,59,123,71,59,129,28,40,41,74,133,42,35,84,97,76,19,110,29,217,53,78,117,76,76,31,80,47,80,67,149,167,114,61,60,283,139,221,207,158,209,89,228,27,13,74,29,26,45,10,113,30,91,69,68,93,63,59,125,70,46,120,27,39,40,73,130,42,37,84,123,75,18,110,47,217,52,77,117,75,76,31,79,47,80,65,150,167,115,62,60,284,139,221,204,161,207,91,231,29,12,77,29,25,44,10,117,29,90,69,69,93,64,60,124,71,46,120,29,39,43,73,132,42,35,84,97,75,18,110,49,218,52,77,118,75,76,32,81,48,80,64,148,167,119,62,61,284,140,222,203,160,208,90,231,28,12,74,29,28,44,10,113,30,92,70,69,94,64,59,125,70,46,122,28,39,41,74,131,43,35,84,98,79,18,43384,26,219,52,77,117,76,76,32,81,48,82,64,149,169,115,62,60,285,140,221,205,162,206,89,231,28,11,74,29,25,44,10,116,28,90,69,69,93,65,60,123,70,46,118,28,39,41,73,130,42,35,84,97,78,19,111,48,220,53,77,118,75,74,31,83,46,80,67,149,168,114,63,60,286,140,223,206,160,217,93,229] ms Reference run times: [679,87,163,46,62,68,17,207,53,154,112,113,269,144,85,259,89,101,199,72,71,66,111,234,56,36,124,105,73,22,163,56,329,64,157,148,79,148,31,144,70,82,81,215,229,153,105,104,375,204,462,328,182,451,124,269,37,13,77,31,29,53,12,117,33,102,75,74,103,64,63,127,75,55,133,32,43,43,82,150,34,34,85,85,67,15,108,25,194,54,79,95,66,68,29,72,49,73,58,146,168,107,69,66,329,124,261,227,206,205,81,201,28,13,75,29,28,48,14,109,34,93,72,72,108,66,61,133,75,49,132,31,43,43,80,153,45,39,99,136,68,20,109,26,203,56,81,115,64,69,28,71,42,72,58,150,152,99,53,56,278,146,261,222,183,251,96,235,34,13,77,31,29,49,12,108,34,103,75,74,107,66,61,125,79,54,134,32,43,43,83,153,37,33,84,84,68,15,109,31,222,50,69,120,67,76,33,80,49,93,71,162,159,120,69,62,334,124,222,188,185,196,75,211,30,12,79,32,28,50,12,116,35,102,76,74,104,65,64,126,68,52,121,29,36,37,68,127,43,41,99,133,85,24,125,31,230,55,81,95,67,66,29,72,47,74,60,147,151,101,69,65,335,119,222,201,197,216,94,220,34,12,77,29,29,49,13,116,33,91,76,78,105,69,60,129,74,51,130,56,44,43,84,159,41,32,83,85,70,22,110,46,199,55,82,121,66,70,29,72,44,74,59,152,166,125,54,57,280,134,265,222,208,225,99,237,34,10,72,27,24,43,10,109,33,103,76,76,105,67,64,125,79,50,135,27,36,37,68,128,46,40,100,136,68,15,108,50,217,55,69,124,79,77,34,80,50,87,72,166,162,99,55,55,278,153,264,223,185,220,94,235,36,14,77,27,24,44,10,117,35,104,76,75,104,64,65,128,76,53,134,27,35,38,68,128,44,41,100,136,84,24,125,31,233,55,73,86,80,76,34,82,53,90,71,169,167,99,55,58,280,116,223,202,219,202,81,195,28,13,75,30,29,47,15,119,36,96,72,73,104,68,62,132,74,50,131,32,43,44,85,152,37,32,84,84,71,20,110,25,209,49,81,114,66,70,29,73,48,73,61,151,152,104,55,57,281,121,223,195,171,194,80,216,28,10,72,27,23,45,11,109,28,103,76,75,104,67,65,126,77,50,134,32,44,44,85,148,37,33,85,86,68,15,122,38,242,56,70,125,82,77,34,81,51,87,72,163,144,119,55,55,280,150,265,229,168,218,95,236] ms Notice that in the reference run, all times are 3 digits. In the sample run, most times seem to line up with the occasional 5 digit time.
(In reply to comment #0) > Ever since that change, the Vista and OSX 10.6 perf bots started failing. The Vista bot consistently fails, while the OSX 10.6 one is flaky: > > http://build.chromium.org/buildbot/waterfall/builders/Mac10.6%20Perf(1)/builds/4306 http://build.chromium.org/buildbot/perf/mac-release-10.6/morejs/report.html?history=150&rev=50051 http://build.chromium.org/buildbot/perf/mac-release-10.6/moz/report.html?history=150&rev=50051 It's hard to see a regression on these charts, but maybe I'm not great at reading them. > http://build.chromium.org/buildbot/waterfall/builders/Vista%20Perf%20(1)/builds/5397 Similarly, http://build.chromium.org/buildbot/perf/vista-release-dual-core/morejs/report.html?history=100&rev=-1 However, intl1 clearly shows something wacky going on: http://build.chromium.org/buildbot/perf/vista-release-dual-core/intl1/report.html?history=100&rev=-1
> Notice that in the reference run, all times are 3 digits. In the sample run, most times seem to line up with the occasional 5 digit time. Does that correspond to individual pages that we're bad at? @tonyg, would you be willing to investigate?
(In reply to comment #3) > > Notice that in the reference run, all times are 3 digits. In the sample run, most times seem to line up with the occasional 5 digit time. > > Does that correspond to individual pages that we're bad at? I talked to chase about this. He was unclear if they should line up or not. One strange thing is that the number of times is not evenly divisible by the number of urls. > > @tonyg, would you be willing to investigate? I'm trying to investigate now, but haven't gotten any solid leads yet.
(In reply to comment #4) > (In reply to comment #3) > > > Notice that in the reference run, all times are 3 digits. In the sample run, most times seem to line up with the occasional 5 digit time. > > > > Does that correspond to individual pages that we're bad at? > > I talked to chase about this. He was unclear if they should line up or not. > > One strange thing is that the number of times is not evenly divisible by the number of urls. > > > > > @tonyg, would you be willing to investigate? > > I'm trying to investigate now, but haven't gotten any solid leads yet. Finally have a solid local repro. Our local versions of news.163.com and sport24.gr will repro consistently. I'm still determining where/how we stall.
I've got a test case now. It looks like the bug is caused by an external resource inside a <noscript> tag. In some of our page cyclers, the paths for resources loaded inside a <noscript> were not re-written to be on the local file system (which is fine). I'm guessing the old preload scanner skipped <noscript> tags whereas the new does not. If so, I'll put together a patch to make the new preload scanner skip <noscript>s.
You can put the lexer into the RCDATA state to make it eat everything in the <noscript> tag.
Created attachment 59042 [details] Patch
I see. We're wrong both before and after this patch, we're just changing it to be the most usual case... Maybe we should just fix the bug? We need to know whether scripts are enabled to decide whether noscript should enter that state. (I think frames should always enter because we don't have a way to disable frames.)
Created attachment 59047 [details] Patch
Comment on attachment 59047 [details] Patch WebCore/html/HTML5TreeBuilder.h:86 + static bool isScriptingFlagEnabled(Frame* frame); No need for the argument name. WebCore/html/HTML5TreeBuilder.cpp:142 + if (tagName == styleTag || tagName == iframeTag || tagName == xmpTag || tagName == noembedTag || tagName == noframesTag || (tagName == noscriptTag && isScriptingFlagEnabled(frame))) Can we wrap this line? :) Looks good to me. I'd like Adam to look too to make sure scriptController->canExecuteScripts(NotAboutToExecuteScript) is the right flavor of "can we script now" call.
Created attachment 59049 [details] Patch
Comment on attachment 59049 [details] Patch You didn't remove the "frame" argument, but it's not a big deal. :)
Committed r61372: <http://trac.webkit.org/changeset/61372>
I'd like to create a subsequent patch with a LayoutTest for this. The case is something like: <script src="slow-script-to-pause-parser-long-enough-for-preload-scanner.js"></script> <noscript> <img src="existent-image.png" onload="failTest()"> </noscript> However, the PreloadScanner (correctly) doesn't trigger onload for the image. So I need another way to determine if the image was fetched (not loaded). Is that possible in a LayoutTest? Any suggestions?
> However, the PreloadScanner (correctly) doesn't trigger onload for the image. So I need another way to determine if the image was fetched (not loaded). Is that possible in a LayoutTest? Any suggestions? Yes! Eric and I found a way to test the preloader: http://trac.webkit.org/browser/trunk/LayoutTests/fast/preloader