Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(381)

Issue 5451057: code review 5451057: runtime: release unused memory to the OS. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 4 months ago by Sebastien Paolacci
Modified:
8 years, 8 months ago
Reviewers:
CC:
rsc, dvyukov, remyoudompheng, golang-dev
Visibility:
Public.

Description

runtime: release unused memory to the OS. Periodically browse MHeap's freelists for long unused spans and release them if any. Current hardcoded settings: - GC is forced if none occured over the last 2 minutes. - spans are handed back after 5 minutes of uselessness. SysUnused (for Unix) is a wrapper on madvise MADV_DONTNEED on Linux and MADV_FREE on BSDs.

Patch Set 1 #

Patch Set 2 : diff -r 2865cfdd5afc https://code.google.com/p/go/ #

Total comments: 10

Patch Set 3 : diff -r 85e087089edf https://code.google.com/p/go/ #

Total comments: 1

Patch Set 4 : diff -r 113217f9853a https://code.google.com/p/go/ #

Total comments: 5

Patch Set 5 : diff -r 7ec969250bfc https://code.google.com/p/go/ #

Patch Set 6 : diff -r 7ec969250bfc https://code.google.com/p/go/ #

Total comments: 9

Patch Set 7 : diff -r 518f09c59498 https://code.google.com/p/go/ #

Total comments: 2

Patch Set 8 : diff -r 63a6abde14b1 https://code.google.com/p/go/ #

Total comments: 4

Patch Set 9 : diff -r fae148fab2a7 https://code.google.com/p/go/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+117 lines, -12 lines) Patch
M src/pkg/runtime/malloc.h View 1 2 3 4 5 6 7 8 4 chunks +7 lines, -2 lines 0 comments Download
M src/pkg/runtime/mem.go View 1 2 3 4 5 6 7 2 chunks +8 lines, -6 lines 0 comments Download
M src/pkg/runtime/mgc0.c View 1 2 3 4 5 6 3 chunks +8 lines, -0 lines 0 comments Download
M src/pkg/runtime/mheap.c View 1 2 3 4 5 6 7 8 6 chunks +82 lines, -1 line 0 comments Download
M src/pkg/runtime/proc.c View 1 2 3 4 5 6 7 4 chunks +12 lines, -3 lines 0 comments Download

Messages

Total messages: 46
Sebastien Paolacci
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go/
9 years, 4 months ago (2011-11-30 21:31:22 UTC) #1
Sebastien Paolacci
Functional sketch, roughly targeted toward long running (so just prevent prominent memory waste). Hardcoded/subjective parameters ...
9 years, 4 months ago (2011-11-30 21:42:57 UTC) #2
rsc
Hi, Thanks very much for doing this. Before I look at the code, I'd like ...
9 years, 4 months ago (2011-12-01 00:04:45 UTC) #3
Sebastien Paolacci
I'll do that for sure. That's actually what my golang-nuts post (~"Giving back some unused ...
9 years, 4 months ago (2011-12-01 13:02:51 UTC) #4
dvyukov
Generally I like the design. It's nice that it's only a once per 10 sec ...
9 years, 4 months ago (2011-12-03 12:21:27 UTC) #5
dvyukov
Some nitpicks http://codereview.appspot.com/5451057/diff/1002/src/pkg/runtime/mheap.c File src/pkg/runtime/mheap.c (right): http://codereview.appspot.com/5451057/diff/1002/src/pkg/runtime/mheap.c#newcode328 src/pkg/runtime/mheap.c:328: runtime·MHeap_Release() I think we need a better/longer ...
9 years, 4 months ago (2011-12-03 12:38:09 UTC) #6
dvyukov
http://codereview.appspot.com/5451057/diff/1002/src/pkg/runtime/mheap.c File src/pkg/runtime/mheap.c (right): http://codereview.appspot.com/5451057/diff/1002/src/pkg/runtime/mheap.c#newcode344 src/pkg/runtime/mheap.c:344: g->waitreason = "MHeap_Release pause"; It is user-visible, please provide ...
9 years, 4 months ago (2011-12-03 14:20:43 UTC) #7
Sebastien Paolacci
Hello golang-dev@googlegroups.com, rsc@golang.org, dvyukov@google.com (cc: golang-dev@googlegroups.com), Please take another look.
9 years, 4 months ago (2011-12-04 13:07:25 UTC) #8
Sebastien Paolacci
Hello Dmitry, thanks for the review. > src/pkg/runtime/mheap.c:328: runtime·MHeap_Release() > I think we need a ...
9 years, 4 months ago (2011-12-04 13:12:14 UTC) #9
Sebastien Paolacci
For cross-reference, a more literal variant is available in the thread http://groups.google.com/group/golang-nuts/browse_thread/thread/5819498d3fb3bbb1/829dbcda0b091314 (20th msg).
9 years, 4 months ago (2011-12-04 13:14:55 UTC) #10
Sebastien Paolacci
On Sat, Dec 3, 2011 at 1:21 PM, <dvyukov@google.com> wrote: > 1. The algo is ...
9 years, 4 months ago (2011-12-04 14:01:08 UTC) #11
dvyukov
http://codereview.appspot.com/5451057/diff/8/src/pkg/runtime/mheap.c File src/pkg/runtime/mheap.c (right): http://codereview.appspot.com/5451057/diff/8/src/pkg/runtime/mheap.c#newcode420 src/pkg/runtime/mheap.c:420: k += 1; k++ I like k += 1 ...
9 years, 4 months ago (2011-12-07 10:22:13 UTC) #12
dvyukov
On 2011/12/04 14:01:08, Sebastien Paolacci wrote: > On Sat, Dec 3, 2011 at 1:21 PM, ...
9 years, 4 months ago (2011-12-07 10:39:55 UTC) #13
rsc
I was hoping for the discussion to happen in this thread. I see now that ...
9 years, 4 months ago (2011-12-07 22:43:09 UTC) #14
Sebastien Paolacci
Hello Russ, Slightly more intrusive but I'm definitely fine with the idea. What about tagging ...
9 years, 4 months ago (2011-12-08 20:56:35 UTC) #15
Sebastien Paolacci
If I rotate my brain a little bit, I should indeed find that both lastinuse ...
9 years, 4 months ago (2011-12-11 22:36:48 UTC) #16
dvyukov
On Thu, Dec 8, 2011 at 2:43 AM, Russ Cox <rsc@golang.org> wrote: > I was ...
9 years, 4 months ago (2011-12-12 14:20:27 UTC) #17
rsc
On Mon, Dec 12, 2011 at 09:20, Dmitry Vyukov <dvyukov@google.com> wrote: > Can the memory ...
9 years, 4 months ago (2011-12-12 21:51:04 UTC) #18
Sebastien Paolacci
Hello, I'm missing free time right now but I hope to be able to get ...
9 years, 4 months ago (2011-12-14 13:13:30 UTC) #19
Sebastien Paolacci
Hello golang-dev@googlegroups.com, rsc@golang.org, dvyukov@google.com (cc: golang-dev@googlegroups.com), Please take another look.
9 years, 3 months ago (2011-12-19 13:01:16 UTC) #20
Sebastien Paolacci
Hello Russ, Dmitry, I've uploaded a new duration based implementation. Most of the nasty parts ...
9 years, 3 months ago (2011-12-19 13:03:24 UTC) #21
Sebastien Paolacci
#-- Sorry for the previous ill-formatted mail. Hello Russ, Dmitry, I've uploaded a new duration ...
9 years, 3 months ago (2011-12-19 13:10:59 UTC) #22
dvyukov
Please run all the tests with various values of GOMAXPROCS: GOMAXPROCS=N ./run.bach --no-rebuild http://codereview.appspot.com/5451057/diff/14001/src/pkg/runtime/malloc.h File ...
9 years, 3 months ago (2011-12-22 12:50:36 UTC) #23
dvyukov
On 2011/12/19 13:10:59, Sebastien Paolacci wrote: > #-- Sorry for the previous ill-formatted mail. > ...
9 years, 3 months ago (2011-12-22 12:52:06 UTC) #24
dvyukov
http://codereview.appspot.com/5451057/diff/14001/src/pkg/runtime/proc.c File src/pkg/runtime/proc.c (right): http://codereview.appspot.com/5451057/diff/14001/src/pkg/runtime/proc.c#newcode563 src/pkg/runtime/proc.c:563: if (gp->goid == scvg->goid && runtime·sched.gwait == 0 && ...
9 years, 3 months ago (2011-12-22 12:55:39 UTC) #25
rsc
On Thu, Dec 22, 2011 at 07:55, <dvyukov@google.com> wrote: > I need to look more ...
9 years, 3 months ago (2011-12-22 20:23:31 UTC) #26
Sebastien Paolacci
On Thu, Dec 22, 2011 at 1:50 PM, <dvyukov@google.com> wrote: > Please run all the ...
9 years, 3 months ago (2011-12-30 15:36:51 UTC) #27
Sebastien Paolacci
PTAL The scavenger does not piggyback anymore on timer's proc but uses a standalone idling ...
9 years, 3 months ago (2011-12-30 16:54:42 UTC) #28
dvyukov
http://codereview.appspot.com/5451057/diff/25006/src/pkg/runtime/malloc.h File src/pkg/runtime/malloc.h (right): http://codereview.appspot.com/5451057/diff/25006/src/pkg/runtime/malloc.h#newcode317 src/pkg/runtime/malloc.h:317: int64 unusedsince; // First time spotted by gc in ...
9 years, 3 months ago (2012-01-11 17:08:39 UTC) #29
dvyukov
http://codereview.appspot.com/5451057/diff/25006/src/pkg/runtime/proc.c File src/pkg/runtime/proc.c (right): http://codereview.appspot.com/5451057/diff/25006/src/pkg/runtime/proc.c#newcode586 src/pkg/runtime/proc.c:586: if(runtime·sched.grunning == 1 && runtime·sched.gwait == 0) Can't the ...
9 years, 3 months ago (2012-01-11 18:06:19 UTC) #30
dvyukov
http://codereview.appspot.com/5451057/diff/25006/src/pkg/runtime/mheap.c File src/pkg/runtime/mheap.c (right): http://codereview.appspot.com/5451057/diff/25006/src/pkg/runtime/mheap.c#newcode327 src/pkg/runtime/mheap.c:327: // TODO(rsc): IncrementalScavenge() to return memory to OS. remove
9 years, 3 months ago (2012-01-12 14:42:44 UTC) #31
Sebastien Paolacci
src/pkg/runtime/malloc.h:317: int64 unusedsince; // First time > spotted > by gc in MSpanFree state > ...
9 years, 2 months ago (2012-01-14 10:21:22 UTC) #32
Sebastien Paolacci
Hello golang-dev@googlegroups.com, rsc@golang.org, dvyukov@google.com (cc: golang-dev@googlegroups.com), Please take another look.
9 years, 2 months ago (2012-01-14 10:23:15 UTC) #33
dvyukov
other than that LGTM generally it looks like it may work :) leaving for rsc ...
9 years, 2 months ago (2012-01-15 13:43:52 UTC) #34
dvyukov
2012/1/14 Sébastien Paolacci <sebastien.paolacci@gmail.com> > > > http://codereview.appspot.com/5451057/diff/25006/src/pkg/runtime/mheap.c#newcode388 > > src/pkg/runtime/mheap.c:388: if(s->state == MSpanFree && ...
9 years, 2 months ago (2012-01-15 13:45:08 UTC) #35
Sebastien Paolacci
> http://codereview.appspot.com/5451057/diff/31002/src/pkg/runtime/mem.go#newcode39 > src/pkg/runtime/mem.go:39: NextGC uint64 // next run in HeapAlloc > *time* (bytes) > ...
9 years, 2 months ago (2012-01-23 12:43:41 UTC) #36
Sebastien Paolacci
Hello golang-dev@googlegroups.com, rsc@golang.org, dvyukov@google.com (cc: golang-dev@googlegroups.com), Please take another look.
9 years, 2 months ago (2012-01-23 12:44:14 UTC) #37
rsc
Thanks for the reminder. This was on my list but it needed to be bumped ...
9 years, 2 months ago (2012-02-06 18:59:50 UTC) #38
remyoudompheng
http://codereview.appspot.com/5451057/diff/38001/src/pkg/runtime/mheap.c File src/pkg/runtime/mheap.c (right): http://codereview.appspot.com/5451057/diff/38001/src/pkg/runtime/mheap.c#newcode398 src/pkg/runtime/mheap.c:398: k, mstats.heap_inuse>>20, mstats.heap_idle>>20, mstats.heap_sys>>20, I get "warning: format mismatch ...
9 years, 2 months ago (2012-02-07 06:50:22 UTC) #39
Sebastien Paolacci
On Mon, Feb 6, 2012 at 7:59 PM, <rsc@golang.org> wrote: > Thanks for the reminder. ...
9 years, 2 months ago (2012-02-07 13:20:40 UTC) #40
Sebastien Paolacci
On Tue, Feb 7, 2012 at 7:50 AM, <remyoudompheng@gmail.com> wrote: > > http://codereview.appspot.com/5451057/diff/38001/src/pkg/runtime/mheap.c#newcode398 > src/pkg/runtime/mheap.c:398: ...
9 years, 2 months ago (2012-02-07 13:32:04 UTC) #41
Sebastien Paolacci
Hello rsc@golang.org, dvyukov@google.com, remyoudompheng@gmail.com (cc: golang-dev@googlegroups.com), Please take another look.
9 years, 2 months ago (2012-02-07 13:32:56 UTC) #42
remyoudompheng
The constants seem a bit large to see an effect when running programs by hand, ...
9 years, 2 months ago (2012-02-09 22:04:55 UTC) #43
rsc
LGTM I got to play with this last night, and it worked nicely. Thanks very ...
9 years, 1 month ago (2012-02-16 18:29:54 UTC) #44
rsc
*** Submitted as http://code.google.com/p/go/source/detail?r=b96e722aad5a *** runtime: release unused memory to the OS. Periodically browse MHeap's ...
9 years, 1 month ago (2012-02-16 18:30:09 UTC) #45
Sebastien Paolacci
8 years, 8 months ago (2012-08-06 21:36:34 UTC) #46
*** Abandoned ***
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b