Bug 107158

Summary: Add a version of the html-parser benchmark which uses srcdoc instead of document.write so it tests the threaded parser
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: New BugsAssignee: Eric Seidel (no email) <eric>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dominicc, rniwa, tonyg, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 107210    
Bug Blocks: 106127    
Attachments:
Description Flags
Patch
none
Patch for landing
none
Patch for landing
none
Patch for landing webkit.review.bot: commit-queue-

Description Eric Seidel (no email) 2013-01-17 12:29:34 PST
Add a version of the html-parser benchmark which uses srcdoc instead of document.write so it tests the threaded parser
Comment 1 Eric Seidel (no email) 2013-01-17 12:30:30 PST
Created attachment 183245 [details]
Patch
Comment 2 Ryosuke Niwa 2013-01-17 12:34:07 PST
Comment on attachment 183245 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=183245&action=review

> PerformanceTests/ChangeLog:11
> +        * Parser/html-parser-srcdoc.html: Added.

Please skip this test by default for now (add it to PerformanceTests/Skipped) since this test is a duplicate of html-parser.html.

> PerformanceTests/Parser/html-parser-srcdoc.html:16
> +PerfTestRunner.prepareToMeasureValuesAsync({iterationCount: 10, done: onCompletedRun, unit: 'ms'});

Please don't set iterationCount. We should just make 20 measurements.
Comment 3 Ryosuke Niwa 2013-01-17 12:34:28 PST
Comment on attachment 183245 [details]
Patch

r=me provided above changes.
Comment 4 Adam Barth 2013-01-17 12:35:17 PST
Comment on attachment 183245 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=183245&action=review

> PerformanceTests/Parser/html-parser-srcdoc.html:6
> +if (window.internals.settings.setThreadedHTMLParser)

Should we check for window.internals too so that we can run this test outside DumpRenderTree?
Comment 5 Eric Seidel (no email) 2013-01-17 12:41:12 PST
It's not an identical benchmark, I would expect it may get slightly different numbers, but I can add it to Skipped if you like.

I assume we'll still be able to run it with run-perf-tests even though it's "Skipped"?
Comment 6 Ryosuke Niwa 2013-01-17 12:46:44 PST
(In reply to comment #5)
> It's not an identical benchmark, I would expect it may get slightly different numbers, but I can add it to Skipped if you like.
> 
> I assume we'll still be able to run it with run-perf-tests even though it's "Skipped"?

Yes. Just pass --force.
Comment 7 Eric Seidel (no email) 2013-01-17 13:00:55 PST
Created attachment 183250 [details]
Patch for landing
Comment 8 Ryosuke Niwa 2013-01-17 13:32:51 PST
Comment on attachment 183250 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=183250&action=review

> PerformanceTests/Parser/html-parser-srcdoc.html:16
> +PerfTestRunner.prepareToMeasureValuesAsync({iterationCount: 10, done: onCompletedRun, unit: 'ms'});

Please remove iterationCount.
Comment 9 Eric Seidel (no email) 2013-01-17 13:40:11 PST
I don't know how the project functioned before "webkit-patch apply-from-bug". :)
Comment 10 Eric Seidel (no email) 2013-01-17 13:41:11 PST
Created attachment 183263 [details]
Patch for landing
Comment 11 WebKit Review Bot 2013-01-17 14:54:42 PST
Comment on attachment 183263 [details]
Patch for landing

Clearing flags on attachment: 183263

Committed r140051: <http://trac.webkit.org/changeset/140051>
Comment 12 WebKit Review Bot 2013-01-17 14:54:46 PST
All reviewed patches have been landed.  Closing bug.
Comment 13 WebKit Review Bot 2013-01-17 17:55:59 PST
Re-opened since this is blocked by bug 107210
Comment 14 Eric Seidel (no email) 2013-01-17 18:44:43 PST
Created attachment 183347 [details]
Patch for landing
Comment 15 Eric Seidel (no email) 2013-01-17 18:45:44 PST
I skipped the file with the correct case now.  It should be ignored on win/linux as well as mac.
Comment 16 WebKit Review Bot 2013-01-17 19:14:06 PST
Comment on attachment 183347 [details]
Patch for landing

Rejecting attachment 183347 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
rce/WebKit/chromium/webkit/media/crypto/ppapi/cdm --revision 173055 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
56>At revision 173055.

________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://queues.webkit.org/results/15946193
Comment 17 Dominic Cooney 2013-01-17 19:33:46 PST
Oops, based on comments in bug 107210 I relanded this with the correct case in r140088.