Bug 99899 – Turn PageLoad tests into simple performancetests

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Zoltan Horvath]

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.

[Zoltan Horvath]

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 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]

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 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]

(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]

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

[Zoltan Horvath]

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

[Zoltan Horvath]

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

[Zoltan Horvath]

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

[Zoltan Horvath]

#5: http://trac.webkit.org/changeset/132670

[Zoltan Horvath]

#6: http://trac.webkit.org/changeset/132693

[Zoltan Horvath]

#7: http://trac.webkit.org/changeset/132815 last commit

[Zoltan Horvath]

I landed every part of the patch. Closing bug.