1079895 - Endurance tests are failing with "nsIMemoryReporterManager.explicit is not available" in 32bit mode

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect, P1)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Since the new OS X 10.10 box is online I see constant failures for the endurance tests. They are completely failing with:

ERROR | Test Failure | {
   "exception": {
     "message": "Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIMemoryReporterManager.explicit]", 
     "lineNumber": 89, 
     "name": "NS_ERROR_NOT_AVAILABLE"
   }
}

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

We might not be really blocked on the dependening fix in mozrunner. What we should do here is to make sure to wrap a try/catch around `.explicit` in performance.js, and default to 0.

Andrei or Andreea, can someone of you implement that?

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Problem here is that in 32bit mode we cannot retrieve explicit but only resident. We should add this as comment.

Comment 3

[Andrei Eftimie]

Attachment: fix.patch

We fallback to 0 if retrieving the memory fails.
OSX 10.0: http://mozmill-crowd.blargon7.com/#/endurance/report/e5ac82d610ad4751cf2efea91521931c

Comment 4

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 8503057 [details] [diff] [review]
fix.patch

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

Looks good, passing to Henrik.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 8503057 [details] [diff] [review]
fix.patch

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

::: lib/performance.js
@@ +83,5 @@
>     *        whatever the test just did.
>     */
>    addCheckpoint : function PerfTracer_addCheckpoint(aLabel) {
> +    // Bug 1079890
> +    // nsIMemoryReporterManager.explicit is not available on OSX 10.10

nit: explicit is not available for 32bit builds, and this in general not only 10.10.

@@ +87,5 @@
> +    // nsIMemoryReporterManager.explicit is not available on OSX 10.10
> +    try {
> +      var explicit = memMgr.explicit;
> +    } catch(e) {
> +      var explicit = 0;

I would move this block below the result block declaration. Then you can directly update result.explicit, and have an empty catch block.

nit: missing blank after `catch`

Comment 6

[Andrei Eftimie]

Attachment: fix2.patch

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 8503190 [details] [diff] [review]
fix2.patch

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

The OS X 10.10 part in the comment is not necessary. It's really for all versions. I will remove it before its landing. Otherwise looks fine. r=me.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

https://hg.mozilla.org/qa/mozmill-tests/rev/b1d199f491d0 (default)

I will trigger a testrun on staging for 10.10 in a moment to verify how it works.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Works fine and it is passing:
http://mozmill-staging.blargon7.com/#/endurance/report/e5ac82d610ad4751cf2efea915389bf9

I don't think that we should fix our dashboard for it right now, given that we do not run 32bit builds with Mozmill at the moment. But I filed a new issue: https://github.com/mozilla/mozmill-dashboard/issues/126

After creating that I wonder if we should have used -1 as fallback value here. Andrei, what do you think?

Comment 10

[Andrei Eftimie]

(In reply to Henrik Skupin (:whimboo) from comment #9)
> After creating that I wonder if we should have used -1 as fallback value
> here. Andrei, what do you think?

That does sound like a good idea to me. Would be easier to differentiate our special case.

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Ok, so can you please come-up with a follow-up patch? We may have to land this for default and aurora now. For older branches we wanna have a combined backport patch. Thanks.

Comment 12

[Andrei Eftimie]

Attachment: followup.patch

This is for Nightly and Aurora (where the original patch is already checked in)

Comment 13

[Andrei Eftimie]

Attachment: combined_beta_release_esr31.patch

Applies to mozilla-beta, mozilla-release and mozilla-esr31

Comment 14

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 8504527 [details] [diff] [review]
followup.patch

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

Landed: 
http://hg.mozilla.org/qa/mozmill-tests/rev/6f2c8ca1be24 (default)
http://hg.mozilla.org/qa/mozmill-tests/rev/cfbc65fd407e (aurora)

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I still see '0' being reported for memory in the endurance reports view.

Comment 16

[Andrei Eftimie]

(In reply to Henrik Skupin (:whimboo) from comment #15)
> I still see '0' being reported for memory in the endurance reports view.
I've checked mozmill-dashboard and we still see 0 because of the following calculation:
https://github.com/mozilla/mozmill-dashboard/blob/83925c8298a5d54a5486d80ef051ece59ecbf096/_attachments/js/dashboard.js#L1661-L1667
> memory.explicit = {
>   min : Math.round(stats.explicit.min * BYTE_TO_MEGABYTE), // Math.round(-1 * 1/1048576) == -0

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Thanks Andrei. Looks like we need an issue for the dashboard then. Lets get the remaining piece in.

Comment 18

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 8504533 [details] [diff] [review]
combined_beta_release_esr31.patch

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

http://hg.mozilla.org/qa/mozmill-tests/rev/3ca233d8fe44 (beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/b11210b5a5a0 (release)
http://hg.mozilla.org/qa/mozmill-tests/rev/0a0d59ce762a (esr31)

Thanks. So this can be closed, what's left will be handled in mozmill-dashboard.

Comment 19

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I'm totally not sure why this bug has been marked as fixed! The first part of the patch did only land on default and was never backported. Also this should have seen by assignee and the ci duty because all endurance jobs are failing on staging for endurance testruns.

Comment 20

[Andrei Eftimie]

(In reply to Henrik Skupin (:whimboo) from comment #19)
> I'm totally not sure why this bug has been marked as fixed!
No, this is fixed.

> The first part
> of the patch did only land on default and was never backported.
No, it was backported.

> Also this
> should have seen by assignee and the ci duty because all endurance jobs are
> failing on staging for endurance testruns.

Endurance tests are failing on production as well, but that is a different issue.
That is bug 1087202 and will be fixed by mozmill 2.0.9.

On staging we already have 2.0.9, and everything is green:
http://mm-ci-staging.qa.scl3.mozilla.com:8080/job/mozilla-central_endurance/4195/console


I think you saw this build on staging and you freaked out:
http://mm-ci-staging.qa.scl3.mozilla.com:8080/job/mozilla-aurora_endurance/2956/console

Please get your facts straight before spewing blame all over when its not needed:
- check the date of the run and the build (10th August 2014) 33 Aurora -- Aurora is on 35 now
- we don't run endurance jobs on aurora anymore for a couple of months already

Thanks.

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+1]]

This was indeed a wrong alert by missing the date of last run. So sorry for that.