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

RESOLVED FIXED in Firefox 38, Firefox OS v2.1

Status

Testing
Mochitest
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: shu, Assigned: tromey)

Tracking

unspecified
mozilla40
x86_64
Linux
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

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

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

3 years ago
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.
(Reporter)

Comment 1

3 years ago
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)
Comment hidden (obsolete)
Comment hidden (obsolete)
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.
(Assignee)

Comment 7

3 years ago
Created attachment 8587623 [details] [diff] [review]
ensure that memory stats show up in treeherder logs

I did this in a bit of a funny way to provide a spot to comment why
"logger" isn't used.
(Reporter)

Comment 8

3 years ago
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)

Updated

3 years ago
Assignee: nobody → ttromey
Status: NEW → ASSIGNED
(Assignee)

Updated

3 years ago
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+
(Assignee)

Comment 11

3 years ago
Created attachment 8587990 [details] [diff] [review]
ensure that memory stats show up in treeherder logs

This version removes the parameter.  Re-requesting review since it's a
reasonably significant change.
Attachment #8587623 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
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+
(Assignee)

Comment 14

3 years ago
Created attachment 8588068 [details] [diff] [review]
ensure that memory stats show up in treeherder logs

s/let/var/ - thanks
(Assignee)

Updated

3 years ago
Attachment #8587990 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Attachment #8588068 - Flags: review+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/e7b56aed5164
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/cc5bf3663f50
https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/23214f805a5c
status-b2g-v2.1: --- → fixed
status-b2g-v2.2: --- → fixed
status-b2g-master: --- → fixed
status-firefox38: --- → affected
status-firefox39: --- → affected
https://hg.mozilla.org/releases/mozilla-aurora/rev/fa0e7f6203a0
status-firefox39: affected → fixed
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.