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 Bugs | Assignee: | 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
Eric Seidel (no email)
2013-01-17 12:29:34 PST
Created attachment 183245 [details]
Patch
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 on attachment 183245 [details]
Patch
r=me provided above changes.
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? 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"? (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. Created attachment 183250 [details]
Patch for landing
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. I don't know how the project functioned before "webkit-patch apply-from-bug". :) Created attachment 183263 [details]
Patch for landing
Comment on attachment 183263 [details] Patch for landing Clearing flags on attachment: 183263 Committed r140051: <http://trac.webkit.org/changeset/140051> All reviewed patches have been landed. Closing bug. Re-opened since this is blocked by bug 107210 Created attachment 183347 [details]
Patch for landing
I skipped the file with the correct case now. It should be ignored on win/linux as well as mac. 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 Oops, based on comments in bug 107210 I relanded this with the correct case in r140088. |