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

Status: RESOLVED FIXED

Description

[Ben Kelly [:bkelly, not reviewing]]

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?

Comment 1

[Hannes Verschore [:h4writer]]

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.

Comment 2

[Hannes Verschore [:h4writer]]

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

Comment 3

[Hannes Verschore [:h4writer]]

Request tracking for a memory leak.

Comment 4

[Benjamin Bouvier [:bbouvier]]

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

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

¯\_(ツ)_/¯

Comment 5

[Hannes Verschore [:h4writer]]

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]:
/

Comment 6

[Hannes Verschore [:h4writer]]

This is a try run of this on aurora:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5cfede4b22ee3f7a984f9b42e76d9a204e8cb10b

Comment 7

[Julien Cristau [:jcristau]]

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

neuter tracelogger for helper threads to avoid memory leak, aurora53+, beta52+

Comment 8

[Julien Cristau [:jcristau]]

This doesn't sound like it's severe enough to track for release; if it does please provide some more explanation :)

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/cb11989283f9

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/2f4590c26813

Comment 11

[Hannes Verschore [:h4writer]]

Can you confirm it is fixed on aurora?

Bug 1334194 also landed. As a result the next nightly should also contain a fix.

Comment 12

[Ben Kelly [:bkelly, not reviewing]]

I'm sorry, but I didn't have an exact STR.  If I see it again I'll file a new bug.  Thanks.