Bug 107158 - Add a version of the html-parser benchmark which uses srcdoc instead of document.write so it tests the threaded parser
Summary: Add a version of the html-parser benchmark which uses srcdoc instead of docum...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Seidel (no email)
URL:
Keywords:
Depends on: 107210
Blocks: 106127
  Show dependency treegraph
 
Reported: 2013-01-17 12:29 PST by Eric Seidel (no email)
Modified: 2013-01-17 19:34 PST (History)
5 users (show)

See Also:


Attachments
Patch (2.38 KB, patch)
2013-01-17 12:30 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch for landing (2.89 KB, patch)
2013-01-17 13:00 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch for landing (2.87 KB, patch)
2013-01-17 13:41 PST, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch for landing (2.94 KB, patch)
2013-01-17 18:44 PST, Eric Seidel (no email)
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.