1312597 - Try increasing the ThreadStackHelper's initial max buffer size to avoid (chrome script) in BHR pseudostacks

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

Bug 1023461 added the ability to get chrome script URIs in thread hang stacks for BHR.

That's great, except that when looking at BHR data (particularly for bug 1310255), I'm noticing we're getting a lot of (chrome script) entries instead of URIs.

I've messed around with ThreadStackHelper locally, and what I think is going on is that ThreadStackHelper is initializing with too small a buffer. The max buffer size that it starts with is actually 0, and then as thread hangs come in (and smash through that limit), the max buffer size is adjusted for next time.

I'm pretty sure that means that for the first bunch of hangs in a process, the chrome script URIs are going to be missing.

I suggest that we increase the initial buffer size from 0 to something else.

I'm not sure what a good value is though. Locally, I noticed that the max buffer size in my content process stabilized around 340 bytes after a few minutes of use, but I really have no idea what it should be.

Hey jchen, is my above analysis / suspicion in the ballpark? If so, any suggestions on what to set the max buffer size to be initially?

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Is it worth adding some kind of Telemetry probe to see where buffers out in the wild converge for their max buffer size? Or do we think we can make an educated guess on what an appropriate initial max buffer size would be?

Comment 2

[(inactive) Jim Chen [:jchen] [:darchons]]

I think adding a telemetry probe is a good idea, but for the mean time I believe using an empirical value is good enough. The origin concern was there would be a bunch of threads that never hang, so we would waste memory if we pre-allocate a large buffer for each thread, but for several hundred bytes that shouldn't be a concern.

Comment 3

[Mike Conley (:mconley) (:⚙️)]

(In reply to Jim Chen [:jchen] [:darchons] from comment #2)
> I think adding a telemetry probe is a good idea, but for the mean time I
> believe using an empirical value is good enough. The origin concern was
> there would be a bunch of threads that never hang, so we would waste memory
> if we pre-allocate a large buffer for each thread, but for several hundred
> bytes that shouldn't be a concern.

Would 512 bytes be reasonable?

Comment 4

[(inactive) Jim Chen [:jchen] [:darchons]]

Yep, sounds good!

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 1312597 - Increase ThreadStackHelper's initial max buffer size to avoid (chrome script) placeholder in BHR pseudostacks.

Review commit: https://reviewboard.mozilla.org/r/88388/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/88388/

Comment 6

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8804389 [details]
Bug 1312597 - Increase ThreadStackHelper's initial max buffer size to avoid (chrome script) placeholder in BHR pseudostacks.

https://reviewboard.mozilla.org/r/88388/#review87446

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/570182c077ee
Increase ThreadStackHelper's initial max buffer size to avoid (chrome script) placeholder in BHR pseudostacks. r=jchen

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/570182c077ee