Re: runtime: add FreeOSMemory(). (issue 7009044)

75 views
Skip to first unread message

r...@golang.org

unread,
Jan 2, 2013, 5:51:13 PM1/2/13
to sebastien...@gmail.com, minu...@gmail.com, golan...@googlegroups.com, re...@codereview-hr.appspotmail.com
please drop (). from CL description


https://codereview.appspot.com/7009044/diff/3005/api/next.txt
File api/next.txt (right):

https://codereview.appspot.com/7009044/diff/3005/api/next.txt#newcode345
api/next.txt:345: pkg runtime, func FreeOSMemory()
Please do not update api/next.txt except in CLs that do nothing else.
Otherwise we get merge conflicts when we have to cherrypick CLs into
the release branch. You can revert this file for now.

https://codereview.appspot.com/7009044/diff/3005/src/pkg/runtime/mgc0.c
File src/pkg/runtime/mgc0.c (left):

https://codereview.appspot.com/7009044/diff/3005/src/pkg/runtime/mgc0.c#oldcode1
src/pkg/runtime/mgc0.c:1: // Copyright 2009 The Go Authors. All rights
reserved.
Why did you delete this copyright notice?
Please put it back (and keep the 2009).

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

https://codereview.appspot.com/7009044/diff/3005/src/pkg/runtime/mheap.c#newcode363
src/pkg/runtime/mheap.c:363: uint64 tick, gcgrace, spangrace;
gcgrace looks like gcg-race. Please use something like gctime and
spantime if you need to rename the variables.

https://codereview.appspot.com/7009044/diff/3005/src/pkg/runtime/mheap.c#newcode412
src/pkg/runtime/mheap.c:412: runtime·printf("scvg(%d): GC forced\n",
seqid);
See other CL. Please keep using the existing format instead of making
unnecessary print statement changes. scvg%d is fine.

https://codereview.appspot.com/7009044/diff/3005/src/pkg/runtime/mheap.c#newcode438
src/pkg/runtime/mheap.c:438: runtime·printf("scvg(%d): %D MB
released\n", seqid, sumreleased>>20);
Same as in other CL: please preserve the old print statements.

https://codereview.appspot.com/7009044/

dvy...@google.com

unread,
Jan 3, 2013, 4:52:44 AM1/3/13
to sebastien...@gmail.com, minu...@gmail.com, r...@golang.org, golan...@googlegroups.com, re...@codereview-hr.appspotmail.com
https://codereview.appspot.com/7009044/diff/3005/src/pkg/runtime/mheap.c#newcode334
src/pkg/runtime/mheap.c:334: s->npreleased = t->npreleased;
Is this change correct if we coalesce with both previous and next spans?

https://codereview.appspot.com/7009044/

sebastien...@gmail.com

unread,
Jan 6, 2013, 4:38:03 AM1/6/13
to minu...@gmail.com, r...@golang.org, dvy...@google.com, golan...@googlegroups.com, re...@codereview-hr.appspotmail.com
On 2012/12/22 15:28:44, minux wrote:
> On 2012/12/22 15:20:59, Sebastien Paolacci wrote:
> > It is supposed to be applied on top of 7002049, but codereview
insists for
> > grabbing both of them into that CL.
> You can wait until that CL is submitted and then mail this CL.
> (mailing two CLs with dependencies is possible but is very
> error-prone.)

You're right, this CL is indeed a mess. I apologize for that.

Please hold until 7002049 is submitted, I'll rebase/fix then.

Thanks,
Sebastien

https://codereview.appspot.com/7009044/
Reply all
Reply to author
Forward
0 new messages