Collect coverage for Firefox startup (?and shutdown?)

NEW
Unassigned

Status

enhancement
2 years ago
2 years ago

People

(Reporter: ekyle, Unassigned)

Tracking

(Blocks 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

While attempting to accelerate startup times, it may be useful to have a C/C++ code coverage of startup. 

I imagine we should could run a "null" test, where we send a signal to emit coverage counters (like in bug 1363469) upon startup; run no tests; and then shutdwn. ts_paint might be this "null" test.

I will leave it to the experts to define "startup".
Here's a test run with talos: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bca2d12abe12d9cd54ba53afd9fdb56cda18a561

Still some work to be done to get code coverage artifacts from it. :mconley, would you be able to tell me which of these suites your interested in?
Flags: needinfo?(mconley)
Hello gmierz,

Of all of them, ts_paint probably captures what we're looking for the most. That's in the "other-e10s" / "other" suite.
Flags: needinfo?(mconley)
gmierz,

There are few directions to go with this

1) Make a special suite, with just ts_paint, that captures startup(?and shutdown?) coverage
2) Add a switch to all coverage that will produce startup coverage in addition to test coverage
3) if startup coverage is produced, should the coverage counters be reset?  This may be good for #2, maybe it would be good for everything.
Putting this on florian's radar.
Great, thanks! I'll try to get coverage from the 'other-e10s' suite first and see if it can help at all. If it doesn't, adding another test suite for start-up and shutdown coverage would be ideal in my opinion (in these circumstances). If we go the other route instead by getting start-up and shut-down coverage for each test suite, we might either get too much redundant data or it could get interesting because there are intermittent bugs at shutdowns and this could get us code coverage for them (maybe we can do this in later on?).
when running with talos since we are just looking for 'coverage' and not timing numbers, we can:
1) run on a VM
2) run a single cycle, no 20/25 cycles
Depends on: 1372324
We could also use part of the code introduced in bug 1363469 to do this (calling DumpCounters when we're done with startup).
Greg/Marco- is there a chance we could get some data for this bug and resolve it?
Flags: needinfo?(gmierz2)
I'm thinking that our best bet (or quickest) to get this coverage would be by making two talos test suites, one to collect coverage on the start up and a second one to collect it in both. We could subtract the startup coverage from the second test suite to get the shutdown coverage. From what :mconley mentions in comment 2, we could take the other-e10s suite and remove everything but ts_paint and use that as a start.

I'm wondering about how we could use Marco's patch and the first thought that comes to mind is we'd need to have some way for making more than one gcda entry per folder, which I don't think we can do yet. Marco can you confirm or elaborate on what we would need to do to use that patch? Maybe we could do what Kyle suggests in comment 3, #2.
Flags: needinfo?(gmierz2)
Like Greg suggests, I believe we should start by setting up a talos suite with just ts_paint so it generates it's own coverage artifact.

Second, we can look into how we can generate more than one coverage artifact per task; which should allow us to leverage the extra information DumpCounters provides; and enhance our new talos ts_paint suite to provide startup, run, and shutdown stats.
Depends on: 1380505
(In reply to Greg Mierzwinski [:gmierz] from comment #9)
> I'm wondering about how we could use Marco's patch and the first thought
> that comes to mind is we'd need to have some way for making more than one
> gcda entry per folder, which I don't think we can do yet. Marco can you
> confirm or elaborate on what we would need to do to use that patch? Maybe we
> could do what Kyle suggests in comment 3, #2.

We don't need more than one gcda entry per folder, we can just call DumpCounters when we are done with startup and move the gcda files somewhere (these will be the "startup" gcda files). Then, we reset the counters with ResetCounters and we collect the gcda files at the end (which will be the "shutdown" counters).
This way we don't even need to run the test suite twice.

Do we actually need the "shutdown" part? I thought the "startup" was what we were particularly interested in.
Posted patch PatchSplinter Review
With this patch (which I will break down in multiple parts), we will be able to dump and reset coverage counters directly from JavaScript.

So, we could just add something like this:
> let codeCoverage = Cc["@mozilla.org/tools/code-coverage;1"].getService(Ci.nsICodeCoverage);<
> codeCoverage.dumpCounters();

(and move the gcda files somewhere before closing Firefox) to https://dxr.mozilla.org/mozilla-central/source/testing/talos/talos/startup_test/tspaint_test.html.
Marco, that's great! I've made a new test suite to run just ts_paint on linux64-ccov: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d4b1a77e96d9a0ef3266713bc206a35194b35e8a&selectedJob=113893030

We can use that suite as a start to get the exact startup and shutdown coverage.
Right now, we can generate a coverage report for this locally using a test-suite called talos-scov that was run on taskcluster. At [1], you can see this test run. To generate it for yourself, do the following: 

1. Clone this repository: https://github.com/marco-c/firefox-code-coverage
2. Within that folder, run | python codecoverage.py PATH/TO/LOCAL/MOZILLA/SRC/DIR/ BRANCH REVISION --suite talos-scov |
3. After this, the report folder will have been created with 'index.html' within it that can be opened to view the report.

(Most of these instructions are taken from here: https://developer.mozilla.org/en-US/docs/Mozilla/Testing/Measuring_Code_Coverage_on_Firefox). 

[1]: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d4b1a77e96d9a0ef3266713bc206a35194b35e8a
I've got a report generated with the patches from bug 1380659 (and some other hacks to the talos suite that I haven't pushed yet for review) which should contain only the startup path (up to "MozAfterPaint" and after a setTimeout(, 0)).
It was generated with optimizations disabled, so the line information should be more precise.

I will try to upload it, but it's ~90 MB.
(In reply to Marco Castelluccio [:marco] from comment #15)
> I've got a report generated with the patches from bug 1380659 (and some
> other hacks to the talos suite that I haven't pushed yet for review) which
> should contain only the startup path (up to "MozAfterPaint" and after a
> setTimeout(, 0)).
> It was generated with optimizations disabled, so the line information should
> be more precise.
> 
> I will try to upload it, but it's ~90 MB.

This is a one-time thing and will require more work to make it usable to other developers, so in the meantime when you want to generate a report by yourself you should follow the steps Greg mentioned in comment 14.
mconely,

Please tell us if this is satisfactory. There are instructions on how to generate the info yourself (and you can use those instructions on scheduled jobs later) an Marco uploaded a one-time, startup-only, coverage report.
Flags: needinfo?(mconley)
This all sounds great, and I'll happily check out the report... but I can't find it. Did I miss it? Where has it been uploaded? Or do I have to generate it myself?
(In reply to Mike Conley (:mconley) - Digging out of needinfo / review backlog. from comment #18)
> This all sounds great, and I'll happily check out the report... but I can't
> find it. Did I miss it? Where has it been uploaded? Or do I have to generate
> it myself?

I'm uploading it on GitHub, I'll post a link shortly.
Great, thank you!

Hey florian - I _think_ the way we can interpret this as, high percentages of line coverage mean that the code was run during ts_paint. So we can ignore all of the 0% ones. We want to look at high percentages.

So we've kinda got to invert our thinking here if we're going to test this thing's usefulness - look for files and lines that _aren't_ red. Those are the ones that were running.

As an example of something I've found - apparently we run parts of the RSS feed sniffer pretty early on: https://marco-c.github.io/ts_paint_startup_coverage_report/browser/components/feeds/nsFeedSniffer.cpp.gcov.html

Do you think this report would be useful in finding things that don't need to run on start-up?
Flags: needinfo?(mconley) → needinfo?(florian)
(In reply to Mike Conley (:mconley) - Digging out of needinfo / review backlog. from comment #21)
> Hey florian - I _think_ the way we can interpret this as, high percentages
> of line coverage mean that the code was run during ts_paint. So we can
> ignore all of the 0% ones. We want to look at high percentages.

Exactly, or actually anything that is not 0%. Higher percentage only means more lines covered, but not more time spent (e.g. you could find a covered function that does some main thread IO in a file that is otherwise completely uncovered).
(In reply to Mike Conley (:mconley) - Digging out of needinfo / review backlog. from comment #21)

> Do you think this report would be useful in finding things that don't need
> to run on start-up?

Potentially, but I would personally be more interested in startup code coverage for our JS code.
Flags: needinfo?(florian)
You need to log in before you can comment on or make changes to this bug.