Paolo and all,
It seems like the global garbage collector is not doing a good job collecting all the garbages and effectively gst is leaking memory: ... after loading my program: st> ObjectMemory current oldSpaceUsedBytes 860160 ... running my program for a while: "Global garbage collection... done, heap grown" "Global garbage collection... done, heap grown" "Global garbage collection... done, heap grown" "Global garbage collection... done, heap grown" "Global garbage collection... done, heap grown" "Global garbage collection... done, heap grown" "Global garbage collection... done, heap grown" "Global garbage collection... done, heap grown" "Global garbage collection... done, heap grown" "Global garbage collection... done, heap grown" "Global garbage collection... done, heap grown" "Global garbage collection... done, heap grown" st> ObjectMemory current oldSpaceUsedBytes 65335296 st> ObjectMemory current oldSpaceSize 97719216 So the heap keep growing. But most of it are really garbages: st> ObjectMemory compact "Global garbage collection... done" ObjectMemory st> ObjectMemory current oldSpaceUsedBytes 880640 So far the only ways I found to really reclaim memory are either "ObjectMemory compact" or back to back "ObjectMemory globalGarbageCollect"; a single "globalGarbageCollect" won't do. Right now I work around the problem by sprinkle my code with "ObjectMemory compact" but that sounds silly, isn't it? A few more observations: * It happends for both 3.0.3 and 3.0.5. I haven't tried the 3.1 branch. * If I leave my program running for a long time gst will run out of memory and crash. After then hack (inserting compact at various places) my program can practically run forever and heap never grows. * This will only happen when the working data set is more than a few hundred KB. I cannot reproduce it with a small data set. * The rate of leaking is similar between 3.0.3 and 3.0.5; however there is a difference still: 3.0.3 will slow down dramatically when the leaking is high; while 3.0.5's speed is not affected as much. Any idea? Thanks a lot Derek Zhou _______________________________________________ help-smalltalk mailing list [hidden email] http://lists.gnu.org/mailman/listinfo/help-smalltalk |
On Saturday 17 January 2009 01:21:00 pm Derek Zhou wrote:
> So far the only ways I found to really reclaim memory are either > "ObjectMemory compact" or back to back "ObjectMemory globalGarbageCollect"; > a > single "globalGarbageCollect" won't do. Right now I work around the problem > by sprinkle my code with "ObjectMemory compact" but that sounds silly, > isn't it? I debugged it a little and come up with a 2 lines patch that fix this memory leak. The idea is to always finish the sweep before the compacting; Otherwise you may be compacting a heap full of garbage. Worse, in my case I am producing garbage faster than the incremental sweeping can get to them so my heap just keep growing. Please apply. --- smalltalk-3.0.5/libgst/oop.c 2008-10-19 04:48:40.000000000 -0700 +++ smalltalk-3.0.5_new/libgst/oop.c 2009-01-18 23:24:18.000000000 -0800 @@ -808,6 +808,7 @@ if COMMON (p_instance) goto ok; + _gst_finish_incremental_gc (); _gst_compact (0); p_instance = (gst_object) _gst_mem_alloc (_gst_mem.old, size); if UNCOMMON (!p_instance) @@ -1125,6 +1126,7 @@ * 100.0 / old_limit > _gst_mem.grow_threshold_percent) { s = "done, heap compacted"; + _gst_finish_incremental_gc (); _gst_compact (0); } _______________________________________________ help-smalltalk mailing list [hidden email] http://lists.gnu.org/mailman/listinfo/help-smalltalk |
Derek Zhou wrote:
> On Saturday 17 January 2009 01:21:00 pm Derek Zhou wrote: >> So far the only ways I found to really reclaim memory are either >> "ObjectMemory compact" or back to back "ObjectMemory globalGarbageCollect"; >> a >> single "globalGarbageCollect" won't do. Right now I work around the problem >> by sprinkle my code with "ObjectMemory compact" but that sounds silly, >> isn't it? > I debugged it a little and come up with a 2 lines patch that fix this memory > leak. The idea is to always finish the sweep before the compacting; Otherwise > you may be compacting a heap full of garbage. Worse, in my case I am > producing garbage faster than the incremental sweeping can get to them so my > heap just keep growing. Please apply. The patch is good, thanks very much. Paolo > ly. > > --- smalltalk-3.0.5/libgst/oop.c 2008-10-19 04:48:40.000000000 -0700 > +++ smalltalk-3.0.5_new/libgst/oop.c 2009-01-18 23:24:18.000000000 -0800 > @@ -808,6 +808,7 @@ > if COMMON (p_instance) > goto ok; > > + _gst_finish_incremental_gc (); > _gst_compact (0); > p_instance = (gst_object) _gst_mem_alloc (_gst_mem.old, size); > if UNCOMMON (!p_instance) > @@ -1125,6 +1126,7 @@ > * 100.0 / old_limit > _gst_mem.grow_threshold_percent) > { > s = "done, heap compacted"; > + _gst_finish_incremental_gc (); > _gst_compact (0); > } _______________________________________________ help-smalltalk mailing list [hidden email] http://lists.gnu.org/mailman/listinfo/help-smalltalk |
In reply to this post by Derek Zhou
Derek Zhou wrote:
> On Saturday 17 January 2009 01:21:00 pm Derek Zhou wrote: >> So far the only ways I found to really reclaim memory are either >> "ObjectMemory compact" or back to back "ObjectMemory globalGarbageCollect"; >> a >> single "globalGarbageCollect" won't do. Right now I work around the problem >> by sprinkle my code with "ObjectMemory compact" but that sounds silly, >> isn't it? > I debugged it a little and come up with a 2 lines patch that fix this memory > leak. The idea is to always finish the sweep before the compacting; Otherwise > you may be compacting a heap full of garbage. Worse, in my case I am > producing garbage faster than the incremental sweeping can get to them so my > heap just keep growing. Please apply. I actually applied this patch instead, but you must be thanked anyway for the debugging. diff --git a/libgst/oop.c b/libgst/oop.c index fdfb4d2..db11c5a 100644 --- a/libgst/oop.c +++ b/libgst/oop.c @@ -1006,6 +1006,8 @@ _gst_compact (size_t new_heap_limit) } else { + /* Do not copy garbage. */ + _gst_finish_incremental_gc (); _gst_mem.numCompactions++; update_stats (&stats.timeOfLastCompaction, NULL, NULL); } @@ -1063,7 +1065,6 @@ void _gst_global_compact () { _gst_global_gc (0); - _gst_finish_incremental_gc (); _gst_compact (0); } Paolo _______________________________________________ help-smalltalk mailing list [hidden email] http://lists.gnu.org/mailman/listinfo/help-smalltalk |
On Monday 19 January 2009 12:34:02 am Paolo Bonzini wrote:
> I actually applied this patch instead, but you must be thanked anyway > for the debugging. While I was fooling around with the GC code, I found a few more minor issues: * nearly all GCs trigger compaction, which sounds wasteful. * heap limit never shrink. So GC can be slow even if the real memory consumption has come down from a high peak. * the GC statistic is not right. Here is a patch I made to address them: --- orig/libgst/oop.c +++ mod/libgst/oop.c @@ -1120,12 +1121,24 @@ { old_limit = MAX (old_limit, _gst_mem.old->heap_total); - /* Check if it's time to compact the heap. */ + /* if memory is still low, go all the way on sweeping */ if UNCOMMON ((next_allocation + _gst_mem.old->heap_total) * 100.0 / old_limit > _gst_mem.grow_threshold_percent) { - s = "done, heap compacted"; - _gst_compact (0); + int target_limit; + _gst_finish_incremental_gc (); + + /* Check if it's time to compact the heap. Compactions make the most + sense if there were lots of garbage. And the heap limit is shrunk + to avoid excessive garbage accumulation in the next round */ + target_limit = (next_allocation + _gst_mem.old->heap_total) * + (100.0 + _gst_mem.space_grow_rate) / _gst_mem.grow_threshold_percent; + if UNCOMMON ( target_limit < old_limit) + { + s = "done, heap compacted"; + _gst_compact (0); + grow_memory_no_compact (target_limit); + } } /* Check if it's time to grow the heap. */ @@ -1280,10 +1292,13 @@ _gst_mem.live_flags &= ~F_REACHABLE; _gst_mem.live_flags |= F_OLD; - _gst_mem.reclaimedBytesPerGlobalGC = - _gst_mem.factor * stats.reclaimedOldSpaceBytesSinceLastGlobalGC + - (1 - _gst_mem.factor) * _gst_mem.reclaimedBytesPerScavenge; - + if (stats.reclaimedOldSpaceBytesSinceLastGlobalGC) + { + _gst_mem.reclaimedBytesPerGlobalGC = + _gst_mem.factor * stats.reclaimedOldSpaceBytesSinceLastGlobalGC + + (1 - _gst_mem.factor) * _gst_mem.reclaimedBytesPerGlobalGC; + stats.reclaimedOldSpaceBytesSinceLastGlobalGC = 0; + } #ifdef ENABLE_JIT_TRANSLATION /* Go and really free the blocks associated to garbage collected native code. */ _______________________________________________ help-smalltalk mailing list [hidden email] http://lists.gnu.org/mailman/listinfo/help-smalltalk |
> Here is a patch I made to address them:
The patch look okay - it cannot make things worse at least. Do you have memory/performance numbers for your app? Paolo _______________________________________________ help-smalltalk mailing list [hidden email] http://lists.gnu.org/mailman/listinfo/help-smalltalk |
In reply to this post by Paolo Bonzini-2
On Monday 19 January 2009 12:34:02 am you wrote:
> I actually applied this patch instead, but you must be thanked anyway > for the debugging. Another thing is I don't think the _gst_incremental_gc_step function in oop.c is doing what it means to do: void _gst_incremental_gc_step () { OOP oop, firstOOP; int i; for (i = 0, oop = _gst_mem.highest_swept_oop, firstOOP = _gst_mem.last_swept_oop; i <= INCREMENTAL_SWEEP_STEP && --oop > firstOOP; oop->flags &= ~F_REACHABLE) { if (--oop > firstOOP) { finished_incremental_gc (); break; } if (!IS_OOP_VALID_GC (oop)) ... If you look closely there is no practical way it can get to the IS_OOP_VALID_GC check, let alone the actual sweeping. However my attempt of fixing it intuitively caused gst to crash so I don't know... Derek _______________________________________________ help-smalltalk mailing list [hidden email] http://lists.gnu.org/mailman/listinfo/help-smalltalk |
In reply to this post by Paolo Bonzini-2
On Tuesday 20 January 2009 10:16:06 pm you wrote:
> > Here is a patch I made to address them: > > The patch look okay - it cannot make things worse at least. Do you > have memory/performance numbers for your app? > > Paolo It does not make much difference for my toy program so I don't know for sure. The second part of the patch should be a no-brainer, right? Derek _______________________________________________ help-smalltalk mailing list [hidden email] http://lists.gnu.org/mailman/listinfo/help-smalltalk |
Derek Zhou wrote:
> On Tuesday 20 January 2009 10:16:06 pm you wrote: >>> Here is a patch I made to address them: >> The patch look okay - it cannot make things worse at least. Do you >> have memory/performance numbers for your app? >> >> Paolo > > It does not make much difference for my toy program so I don't know for sure. > The second part of the patch should be a no-brainer, right? Yes, and I applied it. But the first one slows down the attached program by a factor of 10. :-( I think you need a separate and more conservative knob for shrinking the heap, or maybe we just need to reevaluate your patch after fixing incremental GC. Paolo Object subclass: #Transformation instanceVariableNames: 'q r s t k' classVariableNames: '' poolDictionaries: '' category: nil ! !Transformation class methodsFor: 'instance creation'! new ^super new initialize ! q: anInteger1 r: anInteger2 s: anInteger3 t: anInteger4 ^(super new) q: anInteger1 r: anInteger2 s: anInteger3 t: anInteger4 ! unity ^self q: 1 r: 0 s: 0 t: 1 ! ! !Transformation methodsFor: 'initialize-release'! initialize q := 0. r := 0. s := 0. t := 0. k := 0. ! ! !Transformation methodsFor: 'accessing'! * aTransformation ^self species q: q * aTransformation q r: q * aTransformation r + (r * aTransformation t) s: s * aTransformation q + (t * aTransformation s) t: s * aTransformation r + (t * aTransformation t) ! extract: anInteger ^(q * anInteger + r) // (s * anInteger + t) ! next k := k +1. q := k. r := 4 * k + 2. s := 0. t := 2 * k + 1. ! q ^q ! r ^r ! s ^s ! t ^t ! q: anInteger1 r: anInteger2 s: anInteger3 t: anInteger4 q := anInteger1. r := anInteger2. s := anInteger3. t := anInteger4. k := 0. ! ! Object subclass: #PiDigitSpigot instanceVariableNames: 'z x inverse' classVariableNames: '' poolDictionaries: '' category: nil ! !PiDigitSpigot class methodsFor: 'instance creation'! new ^super new initialize ! ! !PiDigitSpigot methodsFor: 'initialize-release'! initialize z := Transformation unity. x := Transformation new. inverse := Transformation new. ! ! !PiDigitSpigot methodsFor: 'accessing'! next | y | ^(self isSafe: (y := self digit)) ifTrue: [z := self produce: y. y] ifFalse: [z := self consume: x next. self next] ! ! !PiDigitSpigot methodsFor: 'private'! consume: aTransformation ^z * aTransformation ! digit ^(z extract: 3) floor ! isSafe: aDigit ^aDigit = (z extract: 4) floor ! produce: anInteger inverse q: 10 r: -10 * anInteger s: 0 t: 1. ^inverse * z ! ! | i length n pidigits stream | n := (Smalltalk arguments at: 1 ifAbsent: ['1200']) asInteger. i := 0. length := 10. pidigits := PiDigitSpigot new. stream := ReadWriteStream on: (String new: 30). [n > 0] whileTrue: [ (n < length) ifTrue: [ n timesRepeat: [ stream nextPut: (Character digitValue: pidigits next) ]. n to: length do: [:each| stream space]. i := i + n. ] ifFalse: [ length timesRepeat: [ stream nextPut: (Character digitValue: pidigits next) ]. i := i + length. ]. stream tab nextPut: $:. i printOn: stream. stream nl. Transcript nextPutAll: (stream contents). stream reset. n := n - length. ] ! _______________________________________________ help-smalltalk mailing list [hidden email] http://lists.gnu.org/mailman/listinfo/help-smalltalk |
On Wednesday 21 January 2009 12:10:19 am you wrote:
> Yes, and I applied it. But the first one slows down the attached program > by a factor of 10. :-( I see the problem. One quick fix is not to let heap shrink too low by setting a low limit, such as the size eden. After I did that the reclaimedBytesPerGlobalGC went back to the more reasonable 1~2M range and spead went back to normal. > > I think you need a separate and more conservative knob for shrinking the > heap, or maybe we just need to reevaluate your patch after fixing > incremental GC. > > Paolo --- orig/libgst/oop.c +++ mod/libgst/oop.c @@ -1120,12 +1121,25 @@ { old_limit = MAX (old_limit, _gst_mem.old->heap_total); - /* Check if it's time to compact the heap. */ + /* if memory is still low, go all the way on sweeping */ if UNCOMMON ((next_allocation + _gst_mem.old->heap_total) * 100.0 / old_limit > _gst_mem.grow_threshold_percent) { - s = "done, heap compacted"; - _gst_compact (0); + int target_limit; + _gst_finish_incremental_gc (); + + /* Check if it's time to compact the heap. Compaction make the most + sense if there were lots of garbage. And the heap limit is shrunk + to avoid excessive garbage accumulation in the next round */ + target_limit = MAX(_gst_mem.eden.totalSize, (next_allocation + + _gst_mem.old->heap_total) * (100.0 + _gst_mem.space_grow_rate) / + _gst_mem.grow_threshold_percent); + if UNCOMMON ( target_limit < old_limit) + { + s = "done, heap compacted"; + _gst_compact (0); + grow_memory_no_compact (target_limit); + } } /* Check if it's time to grow the heap. */ _______________________________________________ help-smalltalk mailing list [hidden email] http://lists.gnu.org/mailman/listinfo/help-smalltalk |
> I see the problem. One quick fix is not to let heap shrink too low by setting a low limit, such as the size eden. After I did that the reclaimedBytesPerGlobalGC > went back to the more reasonable 1~2M range and spead went back to normal. This does it. Paolo _______________________________________________ help-smalltalk mailing list [hidden email] http://lists.gnu.org/mailman/listinfo/help-smalltalk |
In reply to this post by Derek Zhou
On 01/21/2009 07:20 AM, Derek Zhou wrote:
> On Monday 19 January 2009 12:34:02 am you wrote: >> I actually applied this patch instead, but you must be thanked anyway >> for the debugging. > Another thing is I don't think the _gst_incremental_gc_step function in > oop.c is doing what it means to do: > void > _gst_incremental_gc_step () > { > OOP oop, firstOOP; > int i; > > for (i = 0, oop = _gst_mem.highest_swept_oop, > firstOOP = _gst_mem.last_swept_oop; > i<= INCREMENTAL_SWEEP_STEP&& --oop> firstOOP; > oop->flags&= ~F_REACHABLE) > { > if (--oop> firstOOP) > { > finished_incremental_gc (); > break; > } > > if (!IS_OOP_VALID_GC (oop)) > ... > If you look closely there is no practical way it can get to the > IS_OOP_VALID_GC check, let alone the actual sweeping. However my attempt > of fixing it intuitively caused gst to crash so I don't know... I think I fixed it now. :-) Just because after six months I had a good reason. :-) The bug was just that _gst_mem.last_swept_oop was totally wrong and basically worked by chance. Yes, I mean it... The reason I went back to it, is that I wanted to remove the usleep (1) almost-busy wait (poll every millisecond) for signals done in VMpr_Processor_yield. The changes would be as follows: 1) when GST is idle, it tries doing an incremental garbage collection step, fifty times per second 2) in addition, if there are any, it runs the idle blocks (ProcessorScheduler>>#idleAdd:) round-robin, also fifty times per second. 3) if there is no idle block and the GC is done, however, it can just sigsuspend until the next signal. I had this ready a while ago except that because of the above bug, it would never finish the incremental GC, and never go sigsuspend. :-( I'll push after some more testing on Swazoo. Paolo _______________________________________________ help-smalltalk mailing list [hidden email] http://lists.gnu.org/mailman/listinfo/help-smalltalk |
> I'll push after some more testing on Swazoo.
As expected there was some fallout, so I pushed some more fixes. The algorithms looks sane now. Thanks to Nicolas Petton for sending me the reproducer image. Paolo _______________________________________________ help-smalltalk mailing list [hidden email] http://lists.gnu.org/mailman/listinfo/help-smalltalk |
On Sun, 23 Aug 2009 11:09:40 +0200
Paolo Bonzini <[hidden email]> wrote: > > I'll push after some more testing on Swazoo. > > As expected there was some fallout, so I pushed some more fixes. The > algorithms looks sane now. I'll run my OnlineTester benchmarks with the new system later today, too. Thanks for improving this central part of gst, s. _______________________________________________ help-smalltalk mailing list [hidden email] http://lists.gnu.org/mailman/listinfo/help-smalltalk |
In reply to this post by Paolo Bonzini-2
Le dimanche 23 août 2009 à 11:09 +0200, Paolo Bonzini a écrit :
> > I'll push after some more testing on Swazoo. > > As expected there was some fallout, so I pushed some more fixes. The > algorithms looks sane now. It works for me now, great! Thanks Paolo Nico _______________________________________________ help-smalltalk mailing list [hidden email] http://lists.gnu.org/mailman/listinfo/help-smalltalk signature.asc (204 bytes) Download Attachment |
In reply to this post by Stefan Schmiedl
On 08/23/2009 12:20 PM, Stefan Schmiedl wrote:
> On Sun, 23 Aug 2009 11:09:40 +0200 > Paolo Bonzini<[hidden email]> wrote: > >>> I'll push after some more testing on Swazoo. >> >> As expected there was some fallout, so I pushed some more fixes. The >> algorithms looks sane now. > > I'll run my OnlineTester benchmarks with the new system later today, too. > > Thanks for improving this central part of gst, It's not improving. It's making it do what was meant to do. I'd rather not have touched it even with a pole, but... Anyway, 0% CPU usage when waiting for connections is more than a good reason to do it. Paolo _______________________________________________ help-smalltalk mailing list [hidden email] http://lists.gnu.org/mailman/listinfo/help-smalltalk |
On Sun, 23 Aug 2009 15:08:24 +0200
Paolo Bonzini <[hidden email]> wrote: > > Thanks for improving this central part of gst, > > It's not improving. It's making it do what was meant to do. Oh ... well, if that's so, then it was about time :-) > I'd rather > not have touched it even with a pole, but... > > Anyway, 0% CPU usage when waiting for connections is more than a good > reason to do it. As an added bonus, I just reran my Iliad benchmarks (with the most recent gst and iliad) and the timings are between 5% and 10% percent better. Memory usage seems a bit improved, too, but I'm not really sure there. s. _______________________________________________ help-smalltalk mailing list [hidden email] http://lists.gnu.org/mailman/listinfo/help-smalltalk |
On 08/25/2009 09:16 PM, Stefan Schmiedl wrote:
> On Sun, 23 Aug 2009 15:08:24 +0200 > Paolo Bonzini<[hidden email]> wrote: > >>> Thanks for improving this central part of gst, >> >> It's not improving. It's making it do what was meant to do. > > Oh ... well, if that's so, then it was about time :-) Ahah :-) > As an added bonus, I just reran my Iliad benchmarks (with the most recent > gst and iliad) and the timings are between 5% and 10% percent better. > Memory usage seems a bit improved, too, but I'm not really sure there. Good, I couldn't have given numbers, but that would be expected since now object allocation is faster -- after the incremental GC finished, it's just a while (!IS_OOP_VALID (oop)) oop++; (Interestingly, one of my first changes to GST ever was to change object allocation to use a linked list. This went away with the new GC, but now that the focus on network application is bigger, the incremental GC behavior becomes more interesting and using a linked list would make sense again. I don't really feel like introducing even more instability, but I have to keep this in mind for the future). Paolo _______________________________________________ help-smalltalk mailing list [hidden email] http://lists.gnu.org/mailman/listinfo/help-smalltalk |
Free forum by Nabble | Edit this page |