1643170 - Add memory reporter for HelperThreads::helperContexts_

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P3)

Description

[Andrew McCreight [:mccr8]]

This is a list of JSContexts. It uses about 30,000 bytes of memory in an empty content process. We should have a memory reporter for them.

Comment 1

[Andrew McCreight [:mccr8]]

My patch hits a protected data assert. Maybe due to JSContext::SizeOfExcludingThis calling cycleDetectorVector() while the context isn't active.

Comment 2

[Jan de Mooij [:jandem]]

For what it's worth, we could easily initialize this more lazily. Most helper thread tasks don't request a JSContext anymore.

We have parsing (will be fixed short-term-ish after Stencil) and some GC tasks. On the other hand, if we can remove this completely later this year maybe we should just wait for that.

Comment 3

[Andrew McCreight [:mccr8]]

(In reply to Jan de Mooij [:jandem] from comment #2)

For what it's worth, we could easily initialize this more lazily. Most helper thread tasks don't request a JSContext anymore.

Thanks, that's an interesting idea. I filed bug 1643368 for it.

Comment 4

[Andrew McCreight [:mccr8]]

With the new regexp reporter, this is 41,600 bytes of memory in a nearly-empty content process.

Comment 5

[Andrew McCreight [:mccr8]]

Attachment: Bug 1643170, part 1 - Add JSContext::sizeOfIncludingThis.

JSContexts aren't stored inline, so having this helper is convenient.

Comment 6

[Andrew McCreight [:mccr8]]

Attachment: Bug 1643170, part 2 - Report memory for HelperThreads::helperContexts_.

This is 41600 bytes of unreported memory in each process.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/07fff448894b
part 1 - Add JSContext::sizeOfIncludingThis. r=jandem
https://hg.mozilla.org/integration/autoland/rev/57cbb3edaf8e
part 2 - Report memory for HelperThreads::helperContexts_. r=jandem

Comment 8

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/07fff448894b
https://hg.mozilla.org/mozilla-central/rev/57cbb3edaf8e