| Summary: | Force GC between PageLoad tests | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Philip Rogers <pdr> | ||||||||||||
| Component: | Tools / Tests | Assignee: | Philip Rogers <pdr> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | abarth, dpranke, fmalita, haraken, mjs, morrita, ojan, rniwa, schenney, webkit.review.bot, zoltan | ||||||||||||
| Priority: | P2 | ||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||
| Hardware: | Unspecified | ||||||||||||||
| OS: | Unspecified | ||||||||||||||
| Bug Depends on: | |||||||||||||||
| Bug Blocks: | 77037 | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Philip Rogers
2012-10-02 14:39:12 PDT
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. |