Closed
Bug 1392511
Opened 7 years ago
Closed 7 years ago
Nursery used bytes profiling info is incorrect
Categories
(Core :: JavaScript: GC, defect)
Core
JavaScript: GC
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: pbone, Assigned: pbone)
References
Details
Attachments
(2 files, 4 obsolete files)
28.16 KB,
application/pdf
|
Details | |
8.63 KB,
patch
|
pbone
:
review+
|
Details | Diff | Splinter Review |
The nursery_used_bytes and consequently the promotion rate presented in the profiling data (and probably the JS_GC_PROFILE_NURSERY output) is incorrect. It shows the total allocated heap capacity and not the amount of used heap.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → pbone
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•7 years ago
|
||
It doesn't affect JS_GC_PROFILE_NURSERY.
Assignee | ||
Comment 2•7 years ago
|
||
The extra field nurseryCapacity in the previousGC struct will be used in the near future.
Attachment #8899691 -
Flags: review?(jcoppeard)
Comment 3•7 years ago
|
||
Comment on attachment 8899691 [details] [diff] [review] Bug 1392511 - Report the correct information for used bytes Review of attachment 8899691 [details] [diff] [review]: ----------------------------------------------------------------- Good spot, but I already tried this in bug 1293239. It caused a bunch of performance regressions (bug 1296272) and had to be backed out again. I think the problem is that if something causes us to collect the nursery when it is small we will see a high promotion rate even if the contents are not going to live long. The high rate makes us pre-tenure these kinds of objects and we don't nursery allocate them any more, which ends up hurting performance. I think the way forward is to make it explicit that we only make the pre-tenuring decision based on a promotion rate calculated as if the nursery had reached its maximum size. I'm all for correcting and expanding the data we report to the profiler.
Attachment #8899691 -
Flags: review?(jcoppeard)
Assignee | ||
Comment 4•7 years ago
|
||
Bug 1392511 - Report the correct information for used bytes Only use the promotion rate to make pre tenuring and nursery size decisions (now that it is calculated correctly and not under-estimated) if the nursery is at least 90% full.
Attachment #8899691 -
Attachment is obsolete: true
Attachment #8900551 -
Flags: review?(jcoppeard)
Assignee | ||
Comment 5•7 years ago
|
||
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b3f222ef8ad5652d1d160800bdb9c6bb90f80dfb Talos: https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=b3f222ef8ad5652d1d160800bdb9c6bb90f80dfb
Comment 6•7 years ago
|
||
Comment on attachment 8900551 [details] [diff] [review] Bug 1392511 - Report the correct information for used bytes Review of attachment 8900551 [details] [diff] [review]: ----------------------------------------------------------------- This generally looks fine, but see the comments in calcPromotionRate(). I'd like to see performance results before giving r+, specifically a comparison of talos and octane results with and without this patch. I couldn't get the talos results comparison to work for me for the run you did, I don't know why. I usually run talos with "-t all". I don't know whether that's something to do with it. ::: js/src/gc/Nursery.cpp @@ +490,4 @@ > > json.property("reason", JS::gcreason::ExplainReason(previousGC.reason)); > json.property("bytes_tenured", previousGC.tenuredBytes); > + json.floatProperty("promotion_rate", calcPromotionRate(NULL), 0); Please use |nullptr| rather than |NULL|. ::: js/src/gc/Nursery.h @@ +366,5 @@ > + * promotion rate is accurate enough (the nursery was full enough) to be > + * used for tenuring and other decisions. > + * > + * I'm expecting inlining to remove the memory access to > + * valid_for_tenuring. You can remove this comment. This is a common pattern, and the perf implications of an extra memory access don't matter either way. @@ +369,5 @@ > + * I'm expecting inlining to remove the memory access to > + * valid_for_tenuring. > + */ > + float > + calcPromotionRate(bool *valid_for_tenuring) const { SpideerMonkey style says this should be 'validForTenuring'. Please move this method body to the .cpp file (inline methods are generally for trivial things only). You can mark it |inline| to hint to the compiler to inline it. @@ +382,5 @@ > + * short circuit for OUT_OF_NURSERY. > + */ > + *valid_for_tenuring = > + previousGC.reason == JS::gcreason::Reason::OUT_OF_NURSERY || > + used * 0.9f > capacity; Should this be |used > capacity * 0.9|? If so do we still need the check for OUT_OF_NURSERY?
Attachment #8900551 -
Flags: review?(jcoppeard) → feedback+
Assignee | ||
Comment 7•7 years ago
|
||
(In reply to Jon Coppeard (:jonco) from comment #6) > Comment on attachment 8900551 [details] [diff] [review] > Bug 1392511 - Report the correct information for used bytes > > Review of attachment 8900551 [details] [diff] [review]: > ----------------------------------------------------------------- > > This generally looks fine, but see the comments in calcPromotionRate(). > > I'd like to see performance results before giving r+, specifically a > comparison of talos and octane results with and without this patch. I > couldn't get the talos results comparison to work for me for the run you > did, I don't know why. I usually run talos with "-t all". I don't know > whether that's something to do with it. Is there a way to automate whole browser / shell Octane tests? Even locally on my system. I selected some talos things I thought seemed relevant. They've updated now, I don't see any obvious regressions, but it only has one sample so they have low confidence. I'll do more runs on the updated patch. > @@ +382,5 @@ > > + * short circuit for OUT_OF_NURSERY. > > + */ > > + *valid_for_tenuring = > > + previousGC.reason == JS::gcreason::Reason::OUT_OF_NURSERY || > > + used * 0.9f > capacity; > > Should this be |used > capacity * 0.9|? If so do we still need the check > for OUT_OF_NURSERY? I was attempting to short circult the more complex calculation, but I'm not sure it's worth it so I've removed that now. I made the other changes as requested.
Assignee | ||
Comment 8•7 years ago
|
||
Attachment #8900551 -
Attachment is obsolete: true
Attachment #8901031 -
Flags: review?(jcoppeard)
Assignee | ||
Comment 9•7 years ago
|
||
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9937fe55c49ced314c3e82e5ddb77033ab5409f4 The previous try had some incremental failures but was otherwise fine. Talos: https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=9937fe55c49ced314c3e82e5ddb77033ab5409f4
Comment 10•7 years ago
|
||
(In reply to Paul Bone [:pbone] from comment #7) The updated patch still has the line: *validForTenuring = used * 0.9f > capacity; Shouldn't it be like this: *validForTenuring = used > capacity * 0.9f; > Is there a way to automate whole browser / shell Octane tests? There's no standard way AFAIK. I usually run the shell octane tests a few times in a loop. Perfherder is showing a few regressions but with only one datapoint for the new run we don't have much confidence in these results. Please do retrigger the talos tests so that we can get a better comparison.
Assignee | ||
Comment 11•7 years ago
|
||
(In reply to Jon Coppeard (:jonco) from comment #10) > (In reply to Paul Bone [:pbone] from comment #7) > > The updated patch still has the line: > > *validForTenuring = used * 0.9f > capacity; > > Shouldn't it be like this: > > *validForTenuring = used > capacity * 0.9f; Yes. I saw that you mentioned this in the previous review, but I didn't understand what it was you were pointing out. I thought it was just the other condition I had there. > > Is there a way to automate whole browser / shell Octane tests? > > There's no standard way AFAIK. I usually run the shell octane tests a few > times in a loop. > > Perfherder is showing a few regressions but with only one datapoint for the > new run we don't have much confidence in these results. Please do retrigger > the talos tests so that we can get a better comparison. I'll have to re-run all the tests due to the above error. New links: try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3ebb29ed361c2bc00419f6c3b0551fa07dc8f6c8 talos: https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=3ebb29ed361c2bc00419f6c3b0551fa07dc8f6c8
Assignee | ||
Comment 12•7 years ago
|
||
Attachment #8901031 -
Attachment is obsolete: true
Attachment #8901031 -
Flags: review?(jcoppeard)
Assignee | ||
Updated•7 years ago
|
Attachment #8901193 -
Flags: review?(jcoppeard)
Comment 13•7 years ago
|
||
I retriggered a bunch of the talos tests to get some more confidence in the results. Here's the link to compare against the exact m-c revision used: https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&originalRevision=2306e153fba9&newProject=try&newRevision=3ebb29ed361c2bc00419f6c3b0551fa07dc8f6c8&framework=1
Comment 14•7 years ago
|
||
Comment on attachment 8901193 [details] [diff] [review] Bug 1392511 - Report the correct information for used bytes Review of attachment 8901193 [details] [diff] [review]: ----------------------------------------------------------------- This seems fine. One possible issue is that we will be less likely to shrink the nursery if we repeatedly collect it before it is full (the promotion rate will be determined to be invalid in this case) and this may lead us to hold onto a large nursery longer than necessary. We'll have to see if that turns out to be a problem. The talos results are looking good. There are a couple of stylo/qr results that show a large positive difference but I think this is likely to be a testing problem rather an related to anything we've done here unfortunately. I'd still like to see a run of the octane benchmarks before this goes in.
Attachment #8901193 -
Flags: review?(jcoppeard) → review+
Assignee | ||
Comment 15•7 years ago
|
||
(In reply to Jon Coppeard (:jonco) from comment #13) > I retriggered a bunch of the talos tests to get some more confidence in the > results. > > Here's the link to compare against the exact m-c revision used: > https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla- > central&originalRevision=2306e153fba9&newProject=try&newRevision=3ebb29ed361c > 2bc00419f6c3b0551fa07dc8f6c8&framework=1 It's not clear exactly which tests I want to pay attention to for exercising SpiderMonkey. I can look up all the symbols and find out, but even that doesn't provide a lot of information. Are there any tests in particular that you've found to be more interesting or relevant?
Assignee | ||
Comment 16•7 years ago
|
||
(In reply to Jon Coppeard (:jonco) from comment #14) > Comment on attachment 8901193 [details] [diff] [review] > Bug 1392511 - Report the correct information for used bytes > > Review of attachment 8901193 [details] [diff] [review]: > ----------------------------------------------------------------- > > This seems fine. > > One possible issue is that we will be less likely to shrink the nursery if > we repeatedly collect it before it is full (the promotion rate will be > determined to be invalid in this case) and this may lead us to hold onto a > large nursery longer than necessary. We'll have to see if that turns out to > be a problem. That's true, we can see that in Telemetry and it seems to be low a lot of the time, we can see if this increases when the patch lands. https://telemetry.mozilla.org/new-pipeline/dist.html#!cumulative=0&end_date=2017-08-28&keys=__none__!__none__!__none__&max_channel_version=nightly%252F57&measure=GC_NURSERY_BYTES&min_channel_version=null&processType=*&product=Firefox&sanitize=1&sort_keys=submissions&start_date=2017-08-02&table=0&trim=1&use_submission_date=0 > The talos results are looking good. There are a couple of stylo/qr results > that show a large positive difference but I think this is likely to be a > testing problem rather an related to anything we've done here unfortunately. > > I'd still like to see a run of the octane benchmarks before this goes in. No problem, I can do that.
Assignee | ||
Comment 17•7 years ago
|
||
Here are the octane results. With 5 runs each (before / after my patch) only one test has a statistically significant difference from the mean, and that's Crypto, with a 2.7% improvement. I can't imagine why and it's very likely that this looses significance if I had more than 5 runs per test. However I think this is enough to say this patch probably doesn't do any harm. Jon, if you're happy can you set checkin-needed?
Flags: needinfo?(jcoppeard)
Comment 18•7 years ago
|
||
(In reply to Paul Bone [:pbone] from comment #17) OK, thanks for running the tests. I agree that the crypto improvement is unlikely to be significant unfortunately.
Flags: needinfo?(jcoppeard)
Updated•7 years ago
|
Keywords: checkin-needed
Comment 19•7 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d7c36348c05b Report the correct information for used bytes. r=jonco
Keywords: checkin-needed
Comment 20•7 years ago
|
||
Backed out for valgrind failure on Linux x64 opt: https://hg.mozilla.org/integration/mozilla-inbound/rev/b6c7cf7bb17cec0cabd7a46719805020546e6444 Push with failure: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=d7c36348c05b4634d7f5ade63470fd6962f141f2&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=127438130&repo=mozilla-inbound [task 2017-08-31T16:32:02.785553Z] 16:32:02 INFO - 0:19.53 --46715-- .. build-id is valid [task 2017-08-31T16:32:02.913898Z] 16:32:02 INFO - 0:19.66 --46710-- REDIR: 0x5cfe970 (libc.so.6:malloc_usable_size) redirected to 0x4c27a97 (malloc_usable_size) [task 2017-08-31T16:32:02.921451Z] 16:32:02 INFO - 0:19.67 ==46710== Warning: set address range perms: large range [0x1ce72eaad000, 0x1ce76eaad000) (noaccess) [task 2017-08-31T16:32:05.869650Z] 16:32:05 INFO - 0:22.61 --46710-- REDIR: 0x5db2510 (libc.so.6:__strncmp_sse42) redirected to 0x4c2b070 (__strncmp_sse42) [task 2017-08-31T16:32:07.902861Z] 16:32:07 INFO - 0:24.65 TEST-UNEXPECTED-FAIL | valgrind-test | Conditional jump or move depends on uninitialised value(s) at calcPromotionRate / js::Nursery::maybeResizeNursery / js::Nursery::collect / js::gc::GCRuntime::minorGC [task 2017-08-31T16:32:07.903077Z] 16:32:07 INFO - 0:24.65 ==46710== Conditional jump or move depends on uninitialised value(s) [task 2017-08-31T16:32:07.903327Z] 16:32:07 INFO - 0:24.65 ==46710== at 0x111A2615: calcPromotionRate (Nursery.cpp:470) [task 2017-08-31T16:32:07.903614Z] 16:32:07 INFO - 0:24.65 ==46710== by 0x111A2615: js::Nursery::maybeResizeNursery(JS::gcreason::Reason) (Nursery.cpp:963) [task 2017-08-31T16:32:07.903945Z] 16:32:07 INFO - 0:24.65 ==46710== by 0x111A749D: js::Nursery::collect(JS::gcreason::Reason) (Nursery.cpp:633) [task 2017-08-31T16:32:07.904233Z] 16:32:07 INFO - 0:24.65 ==46710== by 0x10E92361: js::gc::GCRuntime::minorGC(JS::gcreason::Reason, js::gcstats::PhaseKind) [clone .part.863] (jsgc.cpp:7516) [task 2017-08-31T16:32:07.904520Z] 16:32:07 INFO - 0:24.65 ==46710== by 0x10E924D3: minorGC (jsgc.cpp:7537) [task 2017-08-31T16:32:07.904786Z] 16:32:07 INFO - 0:24.65 ==46710== by 0x10E924D3: evictNursery (GCRuntime.h:1449) [task 2017-08-31T16:32:07.905252Z] 16:32:07 INFO - 0:24.65 ==46710== by 0x10E924D3: JS::AutoDisableGenerationalGC::AutoDisableGenerationalGC(JSContext*) (jsgc.cpp:7537) [task 2017-08-31T16:32:07.905489Z] 16:32:07 INFO - 0:24.65 ==46710== by 0x110159F7: JSRuntime::initSelfHosting(JSContext*) (SelfHosting.cpp:2865) [task 2017-08-31T16:32:07.905776Z] 16:32:07 INFO - 0:24.65 ==46710== by 0x10E56232: JS::InitSelfHostedCode(JSContext*) (jsapi.cpp:652) [task 2017-08-31T16:32:07.906116Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xE180E9F: nsXPConnect::InitStatics() (nsXPConnect.cpp:135) [task 2017-08-31T16:32:07.906373Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xE164148: xpcModuleCtor() (XPCModule.cpp:13) [task 2017-08-31T16:32:07.906687Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xFCC4786: Initialize() (nsLayoutModule.cpp:319) [task 2017-08-31T16:32:07.907009Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xD93425D: nsComponentManagerImpl::KnownModule::Load() (nsComponentManager.cpp:770) [task 2017-08-31T16:32:07.907325Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xD934A01: nsFactoryEntry::GetFactory() (nsComponentManager.cpp:1792) [task 2017-08-31T16:32:07.907703Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xD935000: nsComponentManagerImpl::CreateInstanceByContractID(char const*, nsISupports*, nsID const&, void**) [clone .part.102] (nsComponentManager.cpp:1090) [task 2017-08-31T16:32:07.908000Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xD939C74: CreateInstanceByContractID (nsComponentManager.cpp:1066) [task 2017-08-31T16:32:07.908341Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xD939C74: nsComponentManagerImpl::GetServiceByContractID(char const*, nsID const&, void**) [clone .part.112] (nsComponentManager.cpp:1454) [task 2017-08-31T16:32:07.908753Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xD93A0B0: GetServiceByContractID (nsComponentManagerUtils.cpp:64) [task 2017-08-31T16:32:07.909057Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xD93A0B0: CallGetService (nsComponentManagerUtils.cpp:69) [task 2017-08-31T16:32:07.909383Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xD93A0B0: nsGetServiceByContractID::operator()(nsID const&, void**) const (nsComponentManagerUtils.cpp:280) [task 2017-08-31T16:32:07.909732Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xD8E1C1F: nsCOMPtr_base::assign_from_gs_contractid(nsGetServiceByContractID, nsID const&) (nsCOMPtr.cpp:95) [task 2017-08-31T16:32:07.910012Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xD96D1CD: nsCOMPtr (nsCOMPtr.h:928) [task 2017-08-31T16:32:07.910316Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xD96D1CD: NS_InitXPCOM2.part.173 (XPCOMInit.cpp:711) [task 2017-08-31T16:32:07.910663Z] 16:32:07 INFO - 0:24.65 ==46710== by 0x109E7C43: ScopedXPCOMStartup::Initialize() (nsAppRunner.cpp:1537) [task 2017-08-31T16:32:07.910949Z] 16:32:07 INFO - 0:24.65 ==46710== by 0x109EDCD5: XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) (nsAppRunner.cpp:4805) [task 2017-08-31T16:32:07.911308Z] 16:32:07 INFO - 0:24.65 ==46710== by 0x109EE137: XRE_main(int, char**, mozilla::BootstrapConfig const&) (nsAppRunner.cpp:4904) [task 2017-08-31T16:32:07.911740Z] 16:32:07 INFO - 0:24.65 ==46710== by 0x406158: do_main(int, char**, char**) (nsBrowserApp.cpp:236) [task 2017-08-31T16:32:07.912012Z] 16:32:07 INFO - 0:24.65 ==46710== by 0x40585F: main (nsBrowserApp.cpp:309) [task 2017-08-31T16:32:07.912302Z] 16:32:07 INFO - 0:24.65 ==46710== Uninitialised value was created by a heap allocation [task 2017-08-31T16:32:07.912609Z] 16:32:07 INFO - 0:24.65 ==46710== at 0x4C291BA: malloc (vg_replace_malloc.c:298) [task 2017-08-31T16:32:07.912900Z] 16:32:07 INFO - 0:24.65 ==46710== by 0x10E60E5F: js_malloc (Utility.h:236) [task 2017-08-31T16:32:07.913209Z] 16:32:07 INFO - 0:24.65 ==46710== by 0x10E60E5F: js_new<JSRuntime, JSRuntime*&> (Utility.h:353) [task 2017-08-31T16:32:07.913493Z] 16:32:07 INFO - 0:24.65 ==46710== by 0x10E60E5F: js::NewContext(unsigned int, unsigned int, JSRuntime*) (jscntxt.cpp:151) [task 2017-08-31T16:32:07.913819Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xD8DBF89: mozilla::CycleCollectedJSContext::Initialize(JSRuntime*, unsigned int, unsigned int) (CycleCollectedJSContext.cpp:144) [task 2017-08-31T16:32:07.914131Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xE160B18: XPCJSContext::Initialize(XPCJSContext*) (XPCJSContext.cpp:884) [task 2017-08-31T16:32:07.914434Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xE160DCD: XPCJSContext::NewXPCJSContext(XPCJSContext*) (XPCJSContext.cpp:1020) [task 2017-08-31T16:32:07.914759Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xE180DDD: nsXPConnect::nsXPConnect() (nsXPConnect.cpp:74) [task 2017-08-31T16:32:07.915040Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xE180E4D: nsXPConnect::InitStatics() (nsXPConnect.cpp:121) [task 2017-08-31T16:32:07.915361Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xE164148: xpcModuleCtor() (XPCModule.cpp:13) [task 2017-08-31T16:32:07.915682Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xFCC4786: Initialize() (nsLayoutModule.cpp:319) [task 2017-08-31T16:32:07.915981Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xD93425D: nsComponentManagerImpl::KnownModule::Load() (nsComponentManager.cpp:770) [task 2017-08-31T16:32:07.916298Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xD934A01: nsFactoryEntry::GetFactory() (nsComponentManager.cpp:1792) [task 2017-08-31T16:32:07.916652Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xD935000: nsComponentManagerImpl::CreateInstanceByContractID(char const*, nsISupports*, nsID const&, void**) [clone .part.102] (nsComponentManager.cpp:1090) [task 2017-08-31T16:32:07.916932Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xD939C74: CreateInstanceByContractID (nsComponentManager.cpp:1066) [task 2017-08-31T16:32:07.917219Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xD939C74: nsComponentManagerImpl::GetServiceByContractID(char const*, nsID const&, void**) [clone .part.112] (nsComponentManager.cpp:1454) [task 2017-08-31T16:32:07.917511Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xD93A0B0: GetServiceByContractID (nsComponentManagerUtils.cpp:64) [task 2017-08-31T16:32:07.917807Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xD93A0B0: CallGetService (nsComponentManagerUtils.cpp:69) [task 2017-08-31T16:32:07.918124Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xD93A0B0: nsGetServiceByContractID::operator()(nsID const&, void**) const (nsComponentManagerUtils.cpp:280) [task 2017-08-31T16:32:07.918403Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xD8E1C1F: nsCOMPtr_base::assign_from_gs_contractid(nsGetServiceByContractID, nsID const&) (nsCOMPtr.cpp:95) [task 2017-08-31T16:32:07.918728Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xD96D1CD: nsCOMPtr (nsCOMPtr.h:928) [task 2017-08-31T16:32:07.919028Z] 16:32:07 INFO - 0:24.65 ==46710== by 0xD96D1CD: NS_InitXPCOM2.part.173 (XPCOMInit.cpp:711) [task 2017-08-31T16:32:07.919345Z] 16:32:07 INFO - 0:24.65 ==46710== by 0x109E7C43: ScopedXPCOMStartup::Initialize() (nsAppRunner.cpp:1537) [task 2017-08-31T16:32:07.919647Z] 16:32:07 INFO - 0:24.65 ==46710== by 0x109EDCD5: XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) (nsAppRunner.cpp:4805) [task 2017-08-31T16:32:07.919949Z] 16:32:07 INFO - 0:24.65 ==46710== by 0x109EE137: XRE_main(int, char**, mozilla::BootstrapConfig const&) (nsAppRunner.cpp:4904) [task 2017-08-31T16:32:07.920267Z] 16:32:07 INFO - 0:24.65 ==46710== by 0x406158: do_main(int, char**, char**) (nsBrowserApp.cpp:236) [task 2017-08-31T16:32:07.920546Z] 16:32:07 INFO - 0:24.65 ==46710== by 0x40585F: main (nsBrowserApp.cpp:309) [task 2017-08-31T16:32:07.920867Z] 16:32:07 INFO - 0:24.65 ==46710== [task 2017-08-31T16:32:07.921171Z] 16:32:07 INFO - 0:24.65 { [task 2017-08-31T16:32:07.921534Z] 16:32:07 INFO - 0:24.65 <insert_a_suppression_name_here> [task 2017-08-31T16:32:07.921837Z] 16:32:07 INFO - 0:24.65 Memcheck:Cond [task 2017-08-31T16:32:07.922146Z] 16:32:07 INFO - 0:24.65 fun:calcPromotionRate [task 2017-08-31T16:32:07.922477Z] 16:32:07 INFO - 0:24.65 fun:_ZN2js7Nursery18maybeResizeNurseryEN2JS8gcreason6ReasonE [task 2017-08-31T16:32:07.922784Z] 16:32:07 INFO - 0:24.65 fun:_ZN2js7Nursery7collectEN2JS8gcreason6ReasonE [task 2017-08-31T16:32:07.923090Z] 16:32:07 INFO - 0:24.65 fun:_ZN2js2gc9GCRuntime7minorGCEN2JS8gcreason6ReasonENS_7gcstats9PhaseKindE.part.863 [task 2017-08-31T16:32:07.923414Z] 16:32:07 INFO - 0:24.65 fun:minorGC [task 2017-08-31T16:32:07.923709Z] 16:32:07 INFO - 0:24.65 fun:evictNursery [task 2017-08-31T16:32:07.924014Z] 16:32:07 INFO - 0:24.65 fun:_ZN2JS25AutoDisableGenerationalGCC1EP9JSContext [task 2017-08-31T16:32:07.924326Z] 16:32:07 INFO - 0:24.65 fun:_ZN9JSRuntime15initSelfHostingEP9JSContext [task 2017-08-31T16:32:07.924615Z] 16:32:07 INFO - 0:24.65 fun:_ZN2JS18InitSelfHostedCodeEP9JSContext [task 2017-08-31T16:32:07.924878Z] 16:32:07 INFO - 0:24.65 fun:_ZN11nsXPConnect11InitStaticsEv [task 2017-08-31T16:32:07.925230Z] 16:32:07 INFO - 0:24.65 fun:_Z13xpcModuleCtorv [task 2017-08-31T16:32:07.925555Z] 16:32:07 INFO - 0:24.65 fun:_Z10Initializev [task 2017-08-31T16:32:07.925818Z] 16:32:07 INFO - 0:24.65 fun:_ZN22nsComponentManagerImpl11KnownModule4LoadEv [task 2017-08-31T16:32:07.926098Z] 16:32:07 INFO - 0:24.65 fun:_ZN14nsFactoryEntry10GetFactoryEv [task 2017-08-31T16:32:07.926400Z] 16:32:07 INFO - 0:24.65 fun:_ZN22nsComponentManagerImpl26CreateInstanceByContractIDEPKcP11nsISupportsRK4nsIDPPv.part.102 [task 2017-08-31T16:32:07.926705Z] 16:32:07 INFO - 0:24.65 fun:CreateInstanceByContractID [task 2017-08-31T16:32:07.927014Z] 16:32:07 INFO - 0:24.66 fun:_ZN22nsComponentManagerImpl22GetServiceByContractIDEPKcRK4nsIDPPv.part.112 [task 2017-08-31T16:32:07.927342Z] 16:32:07 INFO - 0:24.66 fun:GetServiceByContractID [task 2017-08-31T16:32:07.927640Z] 16:32:07 INFO - 0:24.66 fun:CallGetService [task 2017-08-31T16:32:07.927939Z] 16:32:07 INFO - 0:24.66 fun:_ZNK24nsGetServiceByContractIDclERK4nsIDPPv [task 2017-08-31T16:32:07.928264Z] 16:32:07 INFO - 0:24.66 fun:_ZN13nsCOMPtr_base25assign_from_gs_contractidE24nsGetServiceByContractIDRK4nsID [task 2017-08-31T16:32:07.928511Z] 16:32:07 INFO - 0:24.66 fun:nsCOMPtr [task 2017-08-31T16:32:07.928807Z] 16:32:07 INFO - 0:24.66 fun:NS_InitXPCOM2.part.173 [task 2017-08-31T16:32:07.929343Z] 16:32:07 INFO - 0:24.66 fun:_ZN18ScopedXPCOMStartup10InitializeEv [task 2017-08-31T16:32:07.929402Z] 16:32:07 INFO - 0:24.66 fun:_ZN7XREMain8XRE_mainEiPPcRKN7mozilla15BootstrapConfigE [task 2017-08-31T16:32:07.929688Z] 16:32:07 INFO - 0:24.66 fun:_Z8XRE_mainiPPcRKN7mozilla15BootstrapConfigE [task 2017-08-31T16:32:07.929980Z] 16:32:07 INFO - 0:24.66 fun:_ZL7do_mainiPPcS0_ [task 2017-08-31T16:32:07.930266Z] 16:32:07 INFO - 0:24.66 fun:main [task 2017-08-31T16:32:07.930563Z] 16:32:07 INFO - 0:24.66 } [task 2017-08-31T16:32:07.930902Z] 16:32:07 INFO - 0:24.66 TEST-UNEXPECTED-FAIL | valgrind-test | Conditional jump or move depends on uninitialised value(s) at calcPromotionRate / js::Nursery::maybeResizeNursery / js::Nursery::collect / js::gc::GCRuntime::minorGC
Flags: needinfo?(pbone)
Assignee | ||
Comment 21•7 years ago
|
||
Fixed valgrind failure. r+ carried forward.
Attachment #8901193 -
Attachment is obsolete: true
Flags: needinfo?(pbone)
Attachment #8904107 -
Flags: review+
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Comment 22•7 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/55e08f65ac23 Report the correct information for used bytes. r=jonco
Keywords: checkin-needed
Comment 23•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/55e08f65ac23
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in
before you can comment on or make changes to this bug.
Description
•