runtime: always log GC details in-memory #23815
Comments
|
/cc @aclements |
|
Anyone working on this? If not, I can give this a shot. Prototype: 78ec37e It dumps Should it take buffer size from GODEBUG? |
|
Nobody's working on it that I know of, so feel free. But if MemStats was all we wanted, we could just poll ReadMemStats. Usually we want to know the per-phase CPU/wall time, and that's not in there, just the STW pause wall time. Maybe the easiest way to accomplish this would be to add the extra information to MemStats. That would be covered by the Go compatibility promise though... |
I am assuming there's a typo here. Because the comment says "we cannot change MemStats because of backward compatibility". Or am I missing something? |
|
I presume (didn't verify) that's referring to a field that changed meaning internally, and we can't propagate that change to the public structure because it could break existing code. Adding fields should be fine. |
|
There's an explicit check for the size that would fail on adding a new field. It was added in 2010 : dc9a3b2#diff-17dbb5a92bd43f80a1909a1814e2d42eR82 . AIUI, back then part of code was in C and part in Go; the check was to make sure both sizes matched up. But that's not the case anymore. So, can we safely remove that check? |
|
Thanks for taking this on.
No need for the code to be over engineered by adding GODEBUG flags. Adding
a flag is a very heavy weight change. For context the GC currently has only
1 flag through 10 releases. The code should just pick a reasonable size
and document it as an implementation specific value. If the size is too
small it can be adjusted upward once we understand the use case.
…On Wed, Feb 28, 2018 at 7:11 PM, Dhananjay Nakrani ***@***.*** > wrote:
Anyone working on this? If not, I can give this a shot.
Prototype: 78ec37e
<JayNakrani@78ec37e>
It dumps runtime.MemStats in an internal circular buffer and exposes it
via runtime.ReadAllMemStats(). It doesn't yet expose anything on /debug
handler.
Should it take buffer size from GODEBUG?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#23815 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AA7Wn4TDetKNLbohW8LCGI-NMP6xPMaaks5tZzyrgaJpZM4SEU3A>
.
|
|
Some thoughts on what someone debugging a GC problem would want to see
given a reasonable budget for the buffer.
0. The first 10 cycles.
1. The last 10 GC cycles.
2. Every tenth GC cycle for the last 100 cycles.
3. Every 100th GC cycle for the last 1000 cycles.
4. Every 1000th GC cycle for the last 10,000 cycles.
That's a fixed 50 cycle buffer that covers the last 10K GC cycles.
- Rick
…On Thu, Mar 1, 2018 at 6:03 PM, Rick Hudson ***@***.***> wrote:
Thanks for taking this on.
No need for the code to be over engineered by adding GODEBUG flags. Adding
a flag is a very heavy weight change. For context the GC currently has only
1 flag through 10 releases. The code should just pick a reasonable size
and document it as an implementation specific value. If the size is too
small it can be adjusted upward once we understand the use case.
On Wed, Feb 28, 2018 at 7:11 PM, Dhananjay Nakrani <
***@***.***> wrote:
> Anyone working on this? If not, I can give this a shot.
>
> Prototype: 78ec37e
> <JayNakrani@78ec37e>
>
> It dumps runtime.MemStats in an internal circular buffer and exposes it
> via runtime.ReadAllMemStats(). It doesn't yet expose anything on /debug
> handler.
>
> Should it take buffer size from GODEBUG?
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <#23815 (comment)>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AA7Wn4TDetKNLbohW8LCGI-NMP6xPMaaks5tZzyrgaJpZM4SEU3A>
> .
>
|
|
Change https://golang.org/cl/98335 mentions this issue: |
andybons
added
the
NeedsFix
gopherbot
added
the
compiler/runtime
Whenever a user reports a potential issue with the GC, the first thing we ask for is a GC trace (GODEBUG=gctrace=1). We rarely get it.
gctrace=1 is cheap enough to leave on all the time, but spamming stderr is poor form. Instead, the runtime could log it in memory somewhere, either as text or structured data, and expose it in a /debug handler or something. Then it'd be much easier to pull on demand.
This might also be useful for GODEBUG=sched{detail,trace} but I haven't seen that come up as often.
The text was updated successfully, but these errors were encountered: