Re: SBuf review at r9370

On Wed, Mar 4, 2009 at 9:45 PM, Alex Rousskov
<rousskov_at_measurement-factory.com> wrote:
> On 03/04/2009 06:09 AM, Kinkie wrote:
>> Who'd be in charge of managing the passed memory block?
>> I see two choices:
>> - the caller is in charge
>>   then absorb becomes an alias of assign() and has no reason to exist
>> except create confusion
>> - absorb frees
>>   this would make the behaviour more consistent with the eventual
>> impementation, and a mismanaged pointer will bomb immediately.
>>
>> Do you have a path you'd prefer to see?
>>
> Only "absorb frees" path makes sense, as you pointed out yourself. This
> is why the proposed method is called absorb :-).

Agreed, and implemented.

>> Done. Some resizing may still occur, because:
>> 1- we're not copying the whole MemBlock, just the portion that interests us
>> 2- rounding to memory boundaries due to (eventually) MemPools
>>
> Sure, but all those are under-the-hood things that cow() should not
> worry about.

Yup.

>> estimateCapacity() which right now statically suggests a 20% increase in sizes
>>
> I expect 50% or 100% increase would work better, but this is pure
> speculation on my part and not a big deal. Let's see what others suggest.

Only measuring it once live will make sense, but then I would expect
more sophisticated heuristics will serve us better.

>> MemPools integration will see memAllocationPolicy be replaced by a
>> call to Mem::memAllocateString which will choose the right pool and
>> feed the new size back
>>
> You may want to add the above plan as a source comment.

Already there. Clarified it a bit more. Also, the mempools code is
still in the source (the whole 1 line of it), just disabled via #if
0/#else/#endif

>>> But who supplies the file and line values to the constructor if we just
>>> call throw?
>>>
>> The same way TextException does:
>> throw someException(__FILE__,__LINE__)
>>
> Ah, I see now. Not sure why I missed that before, sorry. BTW, __FILE__
> should be replaced with the new DebugBaseName() or whatever we called
> that thing, I guess. One more reason to enhance Throw() so that it can
> correctly throw custom exceptions.

This is IMO out of scope for this project. Exception code cleanup
should be performed in a specific branch.
If it's fine with everyone, I'd leave as-is for now as it's consistent
with general practice and maybe schedule said cleanup project post-3.2

>> Hm.. How about
>> - slice
>> - shorten
>> - chop
>> - range
>> - cut
>>
> Let's use chop() or, if you prefer, zoom().

chop sounds just right to me. Renamed.

I'd call it a wrap, and move on to review and actual use.

Thanks.

-- 
    /kinkie