runtime: expose total stack usage in MemStats

[randall77]

Non-standard-sized stack allocations should be accounted under StackSys.  Right now they
are treated as regular malloc'd memory.

[dvyukov]

Comment 1:

> Right now they are treated as regular malloc'd memory.
They are not profiled by mprof.
> Non-standard-sized stack allocations should be accounted under StackSys.
If we just account it in StackSys, then part of stacks (std size) will be accounted only
in StackSys, and some part will be accounted both in StackSys and HeapSys. Since you
don't know what is the proportion between these parts, you can't make sense out of
HeapSys and heap profile -- heap profile shows less than HeapSys, but you don't know
what part of that difference is stacks.
mprof profile and memstats must be consistent and allow to verify sanity (tell whether
mprof lies or not).

Labels changed: added release-go1.3, repo-main.

[randall77]

Comment 2:

Dmitry, did your change https://golang.org/cl/72440043 fix this?

[dvyukov]

Comment 3:

No, it did not. That change only fixes g->stacksize, which is an internal counter used
to detect infinite recursion.

[dvyukov]

Comment 4:

StackInuse is also broken (StackInuse decreases during function execution):
go version devel +3cd546ac3a99 Tue Mar 11 16:36:14 2014 +1100 linux/amd64
$ go test -v -cpu 1,1,1,1 runtime -test.run=TestStack
=== RUN TestStackOverflow
--- PASS: TestStackOverflow (0.68 seconds)
=== RUN TestStackSplit
--- PASS: TestStackSplit (0.01 seconds)
=== RUN TestStackMem
--- PASS: TestStackMem (3.09 seconds)
    stack_test.go:1576: Consumed 2MB for stack mem
    stack_test.go:1582: Inuse 0MB for stack mem
=== RUN TestStackOverflow
--- PASS: TestStackOverflow (0.67 seconds)
=== RUN TestStackSplit
--- PASS: TestStackSplit (0.01 seconds)
=== RUN TestStackMem
--- FAIL: TestStackMem (3.10 seconds)
    stack_test.go:1576: Consumed 0MB for stack mem
    stack_test.go:1582: Inuse 17592186044415MB for stack mem
    stack_test.go:1584: Stack inuse: want 4194304, got 18446744073709547520

[rsc]

Comment 5:

StackSys is listed as one of a collection under "Low-level fixed-size structure
allocator statistics." More generally the Sys numbers are supposed to correspond 1:1
with mmap requests made to the system. I don't think we should redefine what
StackSys/StackInuse mean to be different than the other Sys/Inuse pairs. In particular
the sum of all the XxxSys fields needs to match the Sys field.
Maybe instead we can add a new field that counts total stack usage? Maybe just
StackAlloc.
It's fine for Go 1.3 if this is a noninvasive change.

Labels changed: added release-go1.3maybe, removed release-go1.3.

Status changed to Accepted.

[dvyukov]

Comment 6:

> Maybe instead we can add a new field that counts total stack usage? Maybe just
StackAlloc.
We also must be able to understand what is heap size w/o stacks.
There is already StackInuse which currently counts only fixed-size cache allocations.
Maybe we can make StackInuse represent total stack memory (cache + mallocgc), and then
subtract malloced stacks from HeapAlloc?
But this introduces another problem: HeapAlloc and HeapSys won't match...

[rsc]

Comment 7:

It's too confusing to redefine existing counters. Maybe add StackAlloc (all
stack in use) and StackHeap (all stack in use from heap). Then you can use
HeapAlloc - StackHeap to get "heap size without stacks".

[dvyukov]

Comment 8:

For now #7 sounds like the best option... and we will need to introduce mstats paid
certification for developers :)

[dvyukov]

Comment 9:

This issue was updated by revision 00e6fc1.

LGTM=rsc
R=golang-codereviews, bradfitz
CC=golang-codereviews, khr, rsc
https://golang.org/cl/74010043

[rsc]

Comment 10:

Labels changed: added release-go1.4, removed release-go1.3maybe.

[gopherbot]

Comment 11:

CL https://golang.org/cl/104200047 mentions this issue.

[randall77]

Comment 12:

This issue was closed by revision 7c13860.

Status changed to Fixed.

[randall77]

Comment 13:

This issue was closed by revision 3cf83c1.

[gopherbot]

Comment 14:

CL https://golang.org/cl/112240044 mentions this issue.

[randall77]

Comment 15:

This issue was closed by revision f378f30.