Closed
Bug 1369410
Opened 7 years ago
Closed 7 years ago
Enable JSVM code coverage collection on linux64-ccov.
Categories
(Testing :: Code Coverage, enhancement)
Testing
Code Coverage
Tracking
(firefox55 fixed)
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: sparky, Assigned: sparky)
References
(Depends on 3 open bugs, Blocks 1 open bug)
Details
Attachments
(1 file)
Enable JSVM code coverage collection on linux64-ccov. There is a patch ready for this, but there are still jit-test errors left to fix: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f511f3deccc7fc5f9eea7f32dda47293354a0f0f
1. I think 'Script-getOffsetsCoverage-02.js' should be skipped because it expects code coverage to be disabled when it starts. I imagine this test will also be a problem on linux64-jsdcov.
2. 'werror.js', and 'bug1186973.js' test javascript errors. I have a feeling that this has something to do with priorities. The test simply times out and there is no log information.
3. The rest of the tests all fail because the javascript function names are rewritten to a "nice" form before comparison to check if they are lazy. The functions which fail in the comparisons are all lazy functions. The lcov output file 'CodeCoverage.cpp' has "no effect" on this. In other words, when we disable all the collection processes (leaving us with empty .info files) the error is still there so it has nothing to do with the actual collection. So, I believe there is a hook somewhere that's causing this interference. (Something to do with delazification?)
Assignee | ||
Comment 1•7 years ago
|
||
:nbp would you have any ideas about how 2 or 3 could be fixed or where the problems are coming from?
Flags: needinfo?(nicolas.b.pierron)
Assignee | ||
Comment 2•7 years ago
|
||
I spoke with nbp over IRC and he mentioned that problem 3 is expected because we can't relazify after a lazy function is called since we need to keep the coverage count. So, that means that we can skip these tests, along with the tests mentioned in 1.
There is still some work that needs to be done to see if anything can be done for 2.
Flags: needinfo?(nicolas.b.pierron)
Assignee | ||
Comment 3•7 years ago
|
||
For 3, javascript runtime errors are handled everywhere and there does not seem to be any common area that could be used to our advantage. To me, this means that we would have to make a lot of modifications to JIT/IonMonkey to get coverage on purposely failing tests for very little gain.
Joel, I think we can skip all the jit failures. 1 is for coverage tests, which pass when coverage is not running, but fail when it is already running which is expected since the tests expect there to be no code coverage instrumentation enabled when they run. 2 is for the lazy function errors, and as nbp noted, we can't relazify them after because we would lose the coverage counts so these errors need to be skipped. And 3, as I explained above, doesn't provide us with much of a gain in comparison to what we would have to do with JIT/IonMonkey, in theory (I'm going to look into this one some more). What do you think?
Flags: needinfo?(jmaher)
Comment 4•7 years ago
|
||
I am all for skipping tests; lets make sure that we do it in an easy to query way so in the future it is straightforward to find all tests skipped, etc. :)
Flags: needinfo?(jmaher)
Assignee | ||
Comment 5•7 years ago
|
||
I am thinking of adding this comment: # skip-if = coverage for jit-test
That would make it easier to find all tests being skipped for coverage on DXR. I'll make two bugs to document the problems, one for the runtime error failures and one for the lazy function problem. (No bug for the coverage error, since that's self-explanatory).
So I'll skip the tests, write the bugs, test the jsvm on all test suites again and then submit a patch for review.
Assignee | ||
Comment 6•7 years ago
|
||
Here's a test run with JSVM on all test-suites with the tests disabled: https://treeherder.mozilla.org/#/jobs?repo=try&revision=00c60d16bcf1c5233ef2b6b337c3e810dd6d8f89
I can submit the patch now or after bug 1367763 has landed. Either way, one or the other will need to be modified slightly.
Comment 7•7 years ago
|
||
lets submit for review so this can land- the jittests are all green right now!
Comment hidden (mozreview-request) |
Updated•7 years ago
|
Assignee: nobody → gmierz2
Comment 9•7 years ago
|
||
mozreview-review |
Comment on attachment 8873898 [details]
Bug 1369410 - Enable JSVM code coverage collection on linux64-ccov.
https://reviewboard.mozilla.org/r/145286/#review149246
thanks, this looks great.
Attachment #8873898 -
Flags: review?(jmaher) → review+
Comment 10•7 years ago
|
||
Pushed by jmaher@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/74b1f6dafa0a
Enable JSVM code coverage collection on linux64-ccov. r=jmaher
Comment 11•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Updated•7 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•