Closed
Bug 1358201
Opened 8 years ago
Closed 8 years ago
Content processes in e10s builds are not updating the code coverage counters
Categories
(Testing :: Code Coverage, enhancement)
Tracking
(firefox55 fixed)
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: marco, Assigned: marco)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
The content processes in opt builds are terminating by calling "_exit" [1], which doesn't update the coverage counters.
We can fix the problem simply by making linux64-ccov a debug build (where the content processes are terminating by returning from main), which we should probably do anyways.
[1]: http://searchfox.org/mozilla-central/rev/d8ac097a1de2544f0e51d186bc850662c5b7430a/ipc/glue/ProcessChild.cpp#39
Assignee | ||
Comment 1•8 years ago
|
||
Just for reference: http://searchfox.org/mozilla-central/rev/d8ac097a1de2544f0e51d186bc850662c5b7430a/dom/ipc/ContentChild.cpp#2050
_exit is called in debug builds when there's a shutdown hang, but I suppose we don't have to worry about that for code coverage purposes.
Assignee | ||
Comment 2•8 years ago
|
||
This is the first option, making the code-coverage mozconfig import the debug mozconfig instead of the nightly one.
Unfortunately, it doesn't work because code-coverage is disabling jemalloc, which conflicts with dmd or stylo enabled by debug: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9326f3ee6627dcc413a6598c447d4dcf76686133&selectedJob=93077223.
Assignee | ||
Comment 3•8 years ago
|
||
This is the second option, simply adding "ac_add_options --enable-debug" in the code-coverage mozconfig.
This one works: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8c6400fd9e5c1715f681b6ac6cf5d85302046805.
The first option looks cleaner to me, but I'm not sure it's feasible. Why is jemalloc disabled in the code coverage build? Can we remove the --disable-jemalloc option?
Flags: needinfo?(jmaher)
Comment 4•8 years ago
|
||
I am not sure why --disable-jemalloc is in the ccov builds, I don't recall that being required.
Flags: needinfo?(jmaher)
Assignee | ||
Comment 5•8 years ago
|
||
OK, I'll try to push to try without disabling jemalloc and see what happens.
Assignee | ||
Comment 6•8 years ago
|
||
(In reply to Marco Castelluccio [:marco] from comment #5)
> OK, I'll try to push to try without disabling jemalloc and see what happens.
https://treeherder.mozilla.org/logviewer.html#?job_id=93248475&repo=try&lineNumber=16661
The build is failing at 'make -k check'.
/home/worker/workspace/build/src/config/recurse.mk:81: recipe for target 'memory/replace/logalloc/replay/check' failed
So I guess we have to go with the second option for now.
Assignee | ||
Updated•8 years ago
|
Assignee | ||
Comment 7•8 years ago
|
||
I filed bug 1358451 to investigate the failure with jemalloc enabled.
Assignee | ||
Updated•8 years ago
|
Attachment #8860191 -
Flags: review?(jmaher)
Comment 8•8 years ago
|
||
Comment on attachment 8860191 [details] [diff] [review]
Option 2
Review of attachment 8860191 [details] [diff] [review]:
-----------------------------------------------------------------
this looks great!
Attachment #8860191 -
Flags: review?(jmaher) → review+
Assignee | ||
Comment 9•8 years ago
|
||
There's also another option, making the opt build call `exit` instead of `_exit` when MOZ_CODE_COVERAGE is defined.
Maja suggested some tests are disabled in debug builds vs opt builds, so we might want to keep the coverage build an opt build. What are your thoughts? Do we disable many tests or just a few?
Flags: needinfo?(jmaher)
Comment 10•8 years ago
|
||
there are differences in tests on opt vs debug, so it would be hard to get everything running smoothly. I think there are an equal amount running on linux64 opt vs debug, the overlap is very high- so I would pick one platform and go with that.
Flags: needinfo?(jmaher)
Comment 11•8 years ago
|
||
Pushed by mcastelluccio@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5eba3d23afc3
Make linux64-ccov a debug build. r=jmaher
Comment 12•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Comment 13•7 years ago
|
||
I suggest reopening this bug and going for the fix mentioned in comment 9. Simply switching to debug builds is not an adequate fix because it makes sense to use opt builds for coverage analysis for example in fuzzing.
Flags: needinfo?(mcastelluccio)
Assignee | ||
Comment 14•7 years ago
|
||
I'll file a new bug to make coverage opt builds possible, but for now I'd like to keep the CI build a debug build (we have to confirm turning it into an opt build doesn't make us lose some information).
Flags: needinfo?(mcastelluccio)
Assignee | ||
Updated•7 years ago
|
Assignee | ||
Updated•6 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•