Bug 99899

Summary:

Turn PageLoad tests into simple performancetests

Product:

WebKit

Reporter:

Zoltan Horvath <zoltan>

Component:

Tools / Tests

Assignee:

Zoltan Horvath <zoltan>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

pdr, rniwa

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

100029

Bug Blocks:

78984

Attachments:

Description	Flags
cropped proposed patch	zoltan: commit-queue-
light version of the proposed patch	rniwa: review+, zoltan: commit-queue-

Zoltan Horvath

Reported 2012-10-19 18:38:12 PDT

We cannot provide an elegant way to measure the memory consumption of the PageLoad tests, but we can turn them into simple performance tests and measure their memory footprint and performance that way. This patch moves the related svg files to their new location and adds html/js wrappers for them.

Attachments
cropped proposed patch (26.57 KB, patch) 2012-10-19 18:49 PDT, Zoltan Horvath	zoltan: commit-queue-	Details Formatted Diff Diff
light version of the proposed patch (11.27 KB, patch) 2012-10-23 16:57 PDT, Zoltan Horvath	rniwa: review+ zoltan: commit-queue-	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Zoltan Horvath

Comment 1 2012-10-19 18:49:37 PDT

Created attachment 169739 [details] cropped proposed patch I removed the moving of the svg files from the patch since it is more than 20MB.

Ryosuke Niwa

Comment 2 2012-10-20 09:38:27 PDT

Comment on attachment 169739 [details] cropped proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=169739&action=review > PerformanceTests/ChangeLog:12 > + * SVG/33041-Samurai.html: Added. Now that we're renaming all these tests anyway, could you also rename test names to be something more sensible and all CamelCase? > PerformanceTests/ChangeLog:17 > + * SVG/LICENSES: Renamed from PerformanceTests/PageLoad/svg/LICENSES. We probably need this in SVG/resources instead. > PerformanceTests/SVG/42450-under_the_see.html:23 > +PerfTestRunner.measureTime({run: function() { > + var iframe = document.createElement("iframe"); > + iframe.style.width = "600px"; > + iframe.style.height = "800px"; > + > + document.body.appendChild(iframe); > + iframe.contentDocument.open(); > + iframe.contentDocument.write(spec); > + > + // Force the SVG painting. > + var iframeElement = document.getElementsByTagName("iframe")[0]; > + var svg = iframeElement.contentDocument.getElementsByTagName("svg")[0]; > + svg.offsetWidth; > + > + iframe.contentDocument.close(); > + document.body.removeChild(iframe); > +}}); Can we add some sort of a helper method on PerfTestRunner instead of copying & pasting this code in every file? Also, there's no need to call document.getElementsByTagName("iframe")[0] since you already have iframe, and you can probably just do: svg = iframeElement.contentDocument.documentElement getElementsByTagName creates a NodeList on the document and affects the behavior of WebKit so we should try not to use them.

Zoltan Horvath

Comment 3 2012-10-23 16:57:23 PDT

Created attachment 170269 [details] light version of the proposed patch Since the patch is more then 10MB I cannot upload it here, this is the lightwave version.

Ryosuke Niwa

Comment 4 2012-10-23 17:14:56 PDT

Comment on attachment 170269 [details] light version of the proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=170269&action=review > PerformanceTests/resources/runner.js:344 > - this.file = PerfTestRunner.loadFile(test.path); > + test.file = PerfTestRunner.loadFile(test.path); We should probably store this as a local variable instead of attaching it to the test object.

Zoltan Horvath

Comment 5 2012-10-23 17:31:35 PDT

(In reply to comment #4) > (From update of attachment 170269 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=170269&action=review > > > PerformanceTests/resources/runner.js:344 > > - this.file = PerfTestRunner.loadFile(test.path); > > + test.file = PerfTestRunner.loadFile(test.path); > > We should probably store this as a local variable instead of attaching it to the test object. I filed a separate bug for this and I will remove from this patch. https://bugs.webkit.org/show_bug.cgi?id=100172

Zoltan Horvath

Comment 6 2012-10-24 11:49:23 PDT

I commit the patch in pieces: #1: http://trac.webkit.org/changeset/132379

Zoltan Horvath

Comment 7 2012-10-25 11:21:11 PDT

#2: http://trac.webkit.org/changeset/132506

Zoltan Horvath

Comment 8 2012-10-25 14:01:23 PDT

#3: http://trac.webkit.org/changeset/132531

Zoltan Horvath

Comment 9 2012-10-25 16:35:07 PDT

#4: http://trac.webkit.org/changeset/132544