Re: [PATCH 3/5] Add pack-objects window memory usage limit

From: Brian Downing <bdowning@lavos.net>
Date: Jul 12, 2007, 10:02 UTC
Message-ID: <20070712100251.GT4087@lavos.net>
In-Reply-To: <alpine.LFD.0.999.0707112356330.32552@xanadu.home>

On Thu, Jul 12, 2007 at 12:25:54AM -0400, Nicolas Pitre wrote:

Show 17 quoted lines

> On Wed, 11 Jul 2007, Brian Downing wrote:
> 
> > +		while (window_memory_limit &&
> > +		       window_memory_usage > window_memory_limit &&
> > +		       count > 1) {
> > +			uint32_t tail = idx - count;
> > +			if (tail > idx) {
> > +				tail += window + 1;
> > +				tail %= window;
> > +			}
> > +			free_unpacked(array + tail);
> > +			count--;
> > +		}
> 
> This is bogus.  Suppose window = 10 and only array entries 8, 9, 0, 1 
> and 2 are populated.  In that case idx = 2 and count should be 4 (not 
> counting the current entry yet).  You want to evict entry 8.

The current idx has already been depopulated by the time that code is run, and count is probably one higher than you are expecting, so this does actually work.

However, looking at it again, I think if the window hasn't been saturated yet in my current code count will be what you expect in this situation and it will screw up as you describe.

Besides, it is admittedly clumsy as hell (a common affliction when dealing with circular buffers for me it seems). I'll see if I can get something better that works.

Thanks, -bcd