Bug 98203

Created attachment 166755 [details] Force GC between PageLoad tests.

You can see an example of this multi-modal distribution here: http://webkit-perf.appspot.com/graph.html#tests=[[1610801,2001,7288486],[1610801,2001,32196]]&sel=1349148472736.8853,1349214037661,0,422.22222222222223&displayrange=90&datatype=running The light blue area represents the "spikes" once every ~10 tests where DRT ran a GC.

Comment on attachment 166755 [details] Force GC between PageLoad tests. View in context: https://bugs.webkit.org/attachment.cgi?id=166755&action=review > Tools/Scripts/webkitpy/performance_tests/perftest.py:210 > + driver.run_test(DriverInput("resources/forceGC.html", time_out_ms, image_hash=None, should_run_pixel_test=should_run_pixel_test), stop_when_done=False) You can just pass False to should_run_pixel_test. > Tools/Scripts/webkitpy/performance_tests/perftest.py:211 > + return driver.run_test(DriverInput(path_or_url, time_out_ms, image_hash=None, should_run_pixel_test=should_run_pixel_test), stop_when_done=False) Maybe we should call super(ReplayPerfTest, self).run_single instead? > LayoutTests/ChangeLog:16 > + * resources/forceGC.html: Added. How about force-gc.html? or ForceGC.html. I don't think we use camelCase for file names.

Created attachment 166766 [details] Update per reviewer comments

Comment on attachment 166766 [details] Update per reviewer comments Seems odd that this is in LayoutTests instead of PerformanceTests...

Comment on attachment 166766 [details] Update per reviewer comments View in context: https://bugs.webkit.org/attachment.cgi?id=166766&action=review > Tools/Scripts/webkitpy/performance_tests/perftest.py:210 > + super(PageLoadingPerfTest, self).run_single(driver, "resources/force-gc.html", time_out_ms, False) I'm surprised that this works given it's located in LayoutTests. You probably need to give a full path. See the code in perftestrunner.py > LayoutTests/ChangeLog:16 > + * resources/forceGC.html: Added. Oh yeah, this should be in PerformanceTests/resource.

Created attachment 167032 [details] Update per reviewer comments

Comment on attachment 167032 [details] Update per reviewer comments View in context: https://bugs.webkit.org/attachment.cgi?id=167032&action=review > Tools/Scripts/webkitpy/performance_tests/perftest.py:207 > + self.force_gc_file = port.perf_tests_dir() + "/resources/force-gc.html" There's port.host.filesystem.join. Use that instead of +.

Created attachment 167034 [details] Updated to use filesystem.join for joining paths

Thank you both for the reviews. (In reply to comment #5) > (From update of attachment 166766 [details]) > Seems odd that this is in LayoutTests instead of PerformanceTests... This worked since DRT pulls from LayoutTests, but it was a sloppy approach. Moved to PerformanceTests. (In reply to comment #6) > (From update of attachment 166766 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=166766&action=review > > > Tools/Scripts/webkitpy/performance_tests/perftest.py:210 > > + super(PageLoadingPerfTest, self).run_single(driver, "resources/force-gc.html", time_out_ms, False) > > I'm surprised that this works given it's located in LayoutTests. > You probably need to give a full path. See the code in perftestrunner.py Great catch. This probably would have broken on other ports. Fixed! (In reply to comment #8) > (From update of attachment 167032 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=167032&action=review > > > Tools/Scripts/webkitpy/performance_tests/perftest.py:207 > > + self.force_gc_file = port.perf_tests_dir() + "/resources/force-gc.html" > > There's port.host.filesystem.join. Use that instead of +. Done.

Created attachment 167037 [details] Remove unnecessary perf_tests_dir definition.

Comment on attachment 167037 [details] Remove unnecessary perf_tests_dir definition. Clearing flags on attachment: 167037 Committed r130366: <http://trac.webkit.org/changeset/130366>

All reviewed patches have been landed. Closing bug.