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

RESOLVED FIXED in Firefox 55

Status

Testing
Code Coverage
RESOLVED FIXED
6 months ago
6 months ago

People

(Reporter: marco, Assigned: marco)

Tracking

(Blocks: 1 bug)

Version 3
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

6 months ago
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

6 months 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

6 months ago
Created attachment 8860190 [details] [diff] [review]
Option 1

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

6 months ago
Created attachment 8860191 [details] [diff] [review]
Option 2

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)
(Assignee)

Comment 5

6 months ago
OK, I'll try to push to try without disabling jemalloc and see what happens.
(Assignee)

Comment 6

6 months 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

6 months ago
See Also: → bug 1358451
(Assignee)

Comment 7

6 months ago
I filed bug 1358451 to investigate the failure with jemalloc enabled.
(Assignee)

Updated

6 months ago
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+
(Assignee)

Comment 9

6 months 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)
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

6 months 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

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5eba3d23afc3
Status: ASSIGNED → RESOLVED
Last Resolved: 6 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.