still a lot of js::TraceLoggerThread allocated memory in nightly

RESOLVED FIXED

Status

()

Core
JavaScript Engine: JIT
RESOLVED FIXED
9 months ago
8 months ago

People

(Reporter: bkelly, Assigned: h4writer)

Tracking

unspecified
Points:
---

Firefox Tracking Flags

(firefox51- wontfix, firefox52+ fixed, firefox53+ fixed, firefox54 wontfix)

Details

(Whiteboard: [MemShrink])

Attachments

(1 attachment)

(Reporter)

Description

9 months ago
So I've been continuing to run DMD buils in the nightly to try to track down the problem in bug 1330976.  Testing on a build with rev 823dc40ab5fe which includes bug 1334129 I am still seeing about 150MB of TraceLoggerThread memory in use.

It seems all of this is through HandleGCParallelWorkload, in case that matters.

Unreported {
  49,424 blocks in heap block record 1 of 6,696
  50,610,176 bytes (37,957,632 requested / 12,652,544 slop)
  Individual block sizes: 1,024 x 49,424
  19.25% of the heap (19.25% cumulative)
  26.03% of unreported (26.03% cumulative)
  Allocated at {
    #01: calloc_impl (c:\devel\mozilla-central\memory\build\replace_malloc.c:182)
    #02: js::TraceLoggerThread::init (c:\devel\mozilla-central\js\src\vm\tracelogging.cpp:126)
    #03: js::TraceLoggerThreadState::forThread (c:\devel\mozilla-central\js\src\vm\tracelogging.cpp>
    #04: js::TraceLoggerForCurrentThread (c:\devel\mozilla-central\js\src\vm\tracelogging.cpp:1000)
    #05: js::HelperThread::handleGCParallelWorkload (c:\devel\mozilla-central\js\src\vm\helperthrea>
    #06: js::HelperThread::threadLoop (c:\devel\mozilla-central\js\src\vm\helperthreads.cpp:1935)
    #07: js::detail::ThreadTrampoline<void (__cdecl&)(void * __ptr64),js::HelperThread * __ptr64>::>
    #08: o__realloc_base[C:\WINDOWS\System32\ucrtbase.dll +0x1cab0]
    #09: BaseThreadInitThunk[C:\WINDOWS\System32\KERNEL32.DLL +0x8364]
    #10: RtlUserThreadStart[C:\WINDOWS\SYSTEM32\ntdll.dll +0x670d1]
  }
}

Unreported {
  49,378 blocks in heap block record 2 of 6,696
  50,563,072 bytes (37,922,304 requested / 12,640,768 slop)
  Individual block sizes: 1,024 x 49,378
  19.23% of the heap (38.47% cumulative)
  26.00% of unreported (52.03% cumulative)
  Allocated at {
    #01: calloc_impl (c:\devel\mozilla-central\memory\build\replace_malloc.c:182)
    #02: js::TraceLoggerThread::init (c:\devel\mozilla-central\js\src\vm\tracelogging.cpp:128)
    #03: js::TraceLoggerThreadState::forThread (c:\devel\mozilla-central\js\src\vm\tracelogging.cpp>
    #04: js::TraceLoggerForCurrentThread (c:\devel\mozilla-central\js\src\vm\tracelogging.cpp:1000)
    #05: js::HelperThread::handleGCParallelWorkload (c:\devel\mozilla-central\js\src\vm\helperthrea>
    #06: js::HelperThread::threadLoop (c:\devel\mozilla-central\js\src\vm\helperthreads.cpp:1935)
    #07: js::detail::ThreadTrampoline<void (__cdecl&)(void * __ptr64),js::HelperThread * __ptr64>::>
    #08: o__realloc_base[C:\WINDOWS\System32\ucrtbase.dll +0x1cab0]
    #09: BaseThreadInitThunk[C:\WINDOWS\System32\KERNEL32.DLL +0x8364]
    #10: RtlUserThreadStart[C:\WINDOWS\SYSTEM32\ntdll.dll +0x670d1]
  }
}

Unreported {
  49,291 blocks in heap block record 3 of 6,696
  50,473,984 bytes (50,473,984 requested / 0 slop)
  Individual block sizes: 1,024 x 49,291
  19.19% of the heap (57.67% cumulative)
  25.96% of unreported (77.99% cumulative)
  Allocated at {
    #01: malloc_impl (c:\devel\mozilla-central\memory\build\replace_malloc.c:152)
    #02: js::TraceLoggerThread::init (c:\devel\mozilla-central\js\src\vm\tracelogging.cpp:130)
    #03: js::TraceLoggerThreadState::forThread (c:\devel\mozilla-central\js\src\vm\tracelogging.cpp>
    #04: js::TraceLoggerForCurrentThread (c:\devel\mozilla-central\js\src\vm\tracelogging.cpp:1000)
    #05: js::HelperThread::handleGCParallelWorkload (c:\devel\mozilla-central\js\src\vm\helperthrea>
    #06: js::HelperThread::threadLoop (c:\devel\mozilla-central\js\src\vm\helperthreads.cpp:1935)
    #07: js::detail::ThreadTrampoline<void (__cdecl&)(void * __ptr64),js::HelperThread * __ptr64>::>
    #08: o__realloc_base[C:\WINDOWS\System32\ucrtbase.dll +0x1cab0]
    #09: BaseThreadInitThunk[C:\WINDOWS\System32\KERNEL32.DLL +0x8364]
    #10: RtlUserThreadStart[C:\WINDOWS\SYSTEM32\ntdll.dll +0x670d1]
  }
}

Hannes, any thoughts?  Is there anything I can look for in about:memory to determine if we have a lot of runtimes or helper threads?
Flags: needinfo?(hv1989)

Updated

9 months ago
Whiteboard: [MemShrink]
(Assignee)

Comment 1

9 months ago
Bug 1334194 will fix this on nightly.
For aurora and beta I think the best way would be to create patches to not compile tracelogging.
(Assignee)

Comment 2

9 months ago
Created attachment 8833556 [details] [diff] [review]
Patch for aurora/beta

What I said was too harsh. People are using tracelogger on release/beta/aurora. That would have been a no-go.

This patch is better. This will only disable logging the helperthreads. Which is nice to have, but I've only seen people use it in the shell. As a result this is a good temporary trade-off for those releases.

https://hg.mozilla.org/try/pushloghtml?changeset=a4526019942c2184ee8c2021ebbb1d55bcada946
Assignee: nobody → hv1989
Flags: needinfo?(hv1989)
Attachment #8833556 - Flags: review?(bbouvier)
(Assignee)

Comment 3

9 months ago
Request tracking for a memory leak.
status-firefox51: --- → affected
status-firefox52: --- → affected
status-firefox53: --- → affected
tracking-firefox51: --- → ?
tracking-firefox52: --- → ?
tracking-firefox53: --- → ?
Comment on attachment 8833556 [details] [diff] [review]
Patch for aurora/beta

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

¯\_(ツ)_/¯
Attachment #8833556 - Flags: review?(bbouvier) → review+
(Assignee)

Comment 5

8 months ago
Comment on attachment 8833556 [details] [diff] [review]
Patch for aurora/beta

Approval Request Comment
[Feature/Bug causing the regression]:
Tracelogger

[User impact if declined]:
Memory leak throughout the usage of the browser.

[Is this code covered by automated tests?]:
Yes

[Has the fix been verified in Nightly?]:
This is not going to land on nightly. Nightly has another fix which depends on nightly only code.

[Needs manual test from QE? If yes, steps to reproduce]: 
/

[List of other uplifts needed for the feature/fix]:
/

[Is the change risky?]:
[Why is the change risky/not risky?]:
No, it returns "nullptr" for a logger, which has always be the default and all function know that they should treat a nullptr as TraceLogger not being enabled. This has been the default for --disable-trace-logging for many years.

[String changes made/needed]:
/
Attachment #8833556 - Flags: approval-mozilla-beta?
Attachment #8833556 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 6

8 months ago
This is a try run of this on aurora:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5cfede4b22ee3f7a984f9b42e76d9a204e8cb10b
Comment on attachment 8833556 [details] [diff] [review]
Patch for aurora/beta

neuter tracelogger for helper threads to avoid memory leak, aurora53+, beta52+
Attachment #8833556 - Flags: approval-mozilla-beta?
Attachment #8833556 - Flags: approval-mozilla-beta+
Attachment #8833556 - Flags: approval-mozilla-aurora?
Attachment #8833556 - Flags: approval-mozilla-aurora+
This doesn't sound like it's severe enough to track for release; if it does please provide some more explanation :)
status-firefox51: affected → wontfix
tracking-firefox51: ? → -
tracking-firefox52: ? → +
tracking-firefox53: ? → +

Comment 9

8 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/cb11989283f9
status-firefox53: affected → fixed

Comment 10

8 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/2f4590c26813
status-firefox52: affected → fixed
Status: NEW → RESOLVED
Last Resolved: 8 months ago
status-firefox54: --- → wontfix
Resolution: --- → FIXED
(Assignee)

Comment 11

8 months ago
Can you confirm it is fixed on aurora?

Bug 1334194 also landed. As a result the next nightly should also contain a fix.
Flags: needinfo?(bkelly)
(Reporter)

Comment 12

8 months ago
I'm sorry, but I didn't have an exact STR.  If I see it again I'll file a new bug.  Thanks.
Flags: needinfo?(bkelly)
You need to log in before you can comment on or make changes to this bug.