Closed Bug 1358201 Opened 4 years ago Closed 4 years ago

Content processes in e10s builds are not updating the code coverage counters

Categories

(Testing :: Code Coverage, enhancement)

Version 3
enhancement
Not set
normal

Tracking

(firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: marco, Assigned: marco)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

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
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.
Attached patch Option 1Splinter Review
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.
Attached patch Option 2Splinter Review
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)
I am not sure why --disable-jemalloc is in the ccov builds, I don't recall that being required.
Flags: needinfo?(jmaher)
OK, I'll try to push to try without disabling jemalloc and see what happens.
(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.
See Also: → 1358451
I filed bug 1358451 to investigate the failure with jemalloc enabled.
Attachment #8860191 - Flags: review?(jmaher)
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+
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)
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)
https://hg.mozilla.org/mozilla-central/rev/5eba3d23afc3
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
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)
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)
See Also: → 1457391
Depends on: 1457393
See Also: → 1460929
You need to log in before you can comment on or make changes to this bug.