Re: code review 7002049: runtime: earlier detection of unused spans. (issue 7002049)

[sebastien...@gmail.com]

Please take another look.


https://codereview.appspot.com/7002049/diff/1002/src/pkg/runtime/mheap.c
File src/pkg/runtime/mheap.c (right):

https://codereview.appspot.com/7002049/diff/1002/src/pkg/runtime/mheap.c#newcode334
src/pkg/runtime/mheap.c:334: s->npreleased = t->npreleased;
On 2013/01/03 09:46:06, dvyukov wrote:
> Is this change correct if we coalesce with both previous and next
spans?
You're right, thanks!
Done.

https://codereview.appspot.com/7002049/diff/1002/src/pkg/runtime/mheap.c#newcode408
src/pkg/runtime/mheap.c:408: if (trace)
On 2013/01/02 22:46:09, rsc wrote:
> no space between if and (

Done.

https://codereview.appspot.com/7002049/diff/1002/src/pkg/runtime/mheap.c#newcode435
src/pkg/runtime/mheap.c:435: runtime·printf("scvg(%d): %D MB
released\n", k, sumreleased>>20);
On 2013/01/02 22:46:09, rsc wrote:
> Please put this print statement back as it was.
> sumreleased>>20 is a uintptr, so %p not %D
It's actually an amount of released pages, which happened to be an
uintptr for the sake of consistency with MSpan's npages, but which
should also be displayed consistently with the just here-below report
(and gc's one).
Done.

> Also the ( ) are unnecessary.
> Let's keep the CL about functionality and not about print nits.

https://codereview.appspot.com/7002049/diff/1002/src/pkg/runtime/mheap.c#newcode436
src/pkg/runtime/mheap.c:436: runtime·printf("scvg(%d): inuse: %D, idle:
%D, sys: %D, released: ~%D -> consumed: ~%D (MB)\n",
On 2013/01/02 22:46:09, rsc wrote:
> Same.
With the little difference here that accurate accounting can't be
achieved by tracking at the span level (but page) due to merge/split
sequences, so it's also a matter of information correctness.
Same done.

https://codereview.appspot.com/7002049/