Closed Bug 1150646 Opened 5 years ago Closed 5 years ago

INFO MEMORY STAT logging no longer displayed in TreeHerder raw logs for mochitests

Categories

(Testing :: Mochitest, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(firefox38 fixed, firefox39 fixed, firefox40 fixed, b2g-v2.1 fixed, b2g-v2.1S fixed, b2g-v2.2 fixed, b2g-master fixed)

RESOLVED FIXED
mozilla40
Tracking Status
firefox38 --- fixed
firefox39 --- fixed
firefox40 --- fixed
b2g-v2.1 --- fixed
b2g-v2.1S --- fixed
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: shu, Assigned: tromey)

Details

Attachments

(1 file, 3 obsolete files)

A while ago we added printing of memory information after every mochitest. They look something like the following.

75 INFO MEMORY STAT vsize after test: 892674048
76 INFO MEMORY STAT vsizeMaxContiguous not supported in this build configuration.
77 INFO MEMORY STAT residentFast after test: 324292608
78 INFO MEMORY STAT heapAllocated after test: 84140968

The raw logs on TH no longer has these, though if I run a mochitest locally I see them.
Hi Chris, I was told you're the person to talk to about possible logging issues?
Flags: needinfo?(cmanchester)
I think that's suppressed intentionally (along with a bunch of other output from tests at some point deemed mundane). I can probably figure out how to turn it back on. Are you intending to do this on try to debug a one off issue, or do we want this on for everything?
Flags: needinfo?(cmanchester) → needinfo?(shu)
Attachment #8587588 - Attachment is obsolete: true
Attachment #8587588 - Flags: review?(nfroyd)
(In reply to Chris Manchester [:chmanchester] from comment #2)
> Are you intending to do this on try to debug a one off
> issue, or do we want this on for everything?

This is needed on an ongoing basis in order to let people see how close we are to OOMing during tests.  Previously, a variety of leaks and issues came together to result in a multi-day tree closure.
Or quasi-intentionally. Replacing logger.info with dump in MemoryStats.dump should do it. Running mach with --quiet will give you the output you see on treeherder.
I did this in a bit of a funny way to provide a spot to comment why
"logger" isn't used.
100% what mccr8 said.
Flags: needinfo?(shu)
More concretely, the use case is that we start getting weird crashes, then the sheriffs can poke around in the log to see if memory usage is crazy high or not, and then hopefully find somebody to investigate before we get to the point of crisis which closes the tree.
Assignee: nobody → ttromey
Status: NEW → ASSIGNED
Attachment #8587623 - Flags: review?(cmanchester)
Comment on attachment 8587623 [details] [diff] [review]
ensure that memory stats show up in treeherder logs

Review of attachment 8587623 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/mochitest/tests/SimpleTest/MemoryStats.js
@@ +54,3 @@
>  }
>  
>  MemoryStats.dump = function (logger,

Optionally also get rid of this parameter now that it's not used.
Attachment #8587623 - Flags: review?(cmanchester) → review+
This version removes the parameter.  Re-requesting review since it's a
reasonably significant change.
Attachment #8587623 - Attachment is obsolete: true
Attachment #8587990 - Flags: review?(cmanchester)
Comment on attachment 8587990 [details] [diff] [review]
ensure that memory stats show up in treeherder logs

Review of attachment 8587990 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for doing this. r=me with a fix for mochitest-plain.

::: testing/mochitest/tests/SimpleTest/MemoryStats.js
@@ +59,5 @@
>                               dumpAboutMemory,
>                               dumpDMD) {
> +    // Use dump because treeherder uses --quiet, which drops 'info'
> +    // from the structured logger.
> +    let info = function(message) {

This needs to be "var" for mochitest-plain unless we also change the tags that include the script.
Attachment #8587990 - Flags: review?(cmanchester) → review+
s/let/var/ - thanks
Attachment #8587990 - Attachment is obsolete: true
Attachment #8588068 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/e7b56aed5164
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in before you can comment on or make changes to this bug.