Re: [PATCH V2 3/3] strbuf: allow to use preallocated memory

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> Jeff King <peff@peff.net> writes:
>
>>> I think that call should reset line.buf to the original buffer on
>>> the stack, instead of saying "Ok, I'll ignore the original memory
>>> not owned by us and instead keep pointing at the allocated memory",
>>> as the allocation was done as a fallback measure.
>>
>> I am not sure I agree. Do we think accessing the stack buffer is somehow
>> cheaper than the heap buffer (perhaps because of cache effects)? If so,
>> how much cheaper?
>
> This is not about stack vs heap or even "cheaper" (whatever your
> definition of cheap is).  The principle applies equally if the
> original buffer came from BSS.
>
> Perhaps I made it clearer by using a more exaggerated example e.g. a

s/I made/I should have made/; sorry but I have a horrible wifi
around here today...

> typical expected buffer size of 128 bytes, but the third line of
> 1000 line input file was an anomaly that is 200k bytes long.  I do
> not want to keep that 200k bytes after finishing to process that
> third line while using its initial 80 bytes for the remaining 977
> lines.
>
> By the way, William seemed to be unhappy with die(), but I actually
> think having a die() in the API may not be a bad thing if the check
> were about something more sensible.  For example, even if a strbuf
> that can grow dynamically, capping the maximum size and say "Hey
> this is a one-lne-at-a-time text interface; if we need to grow the
> buffer to 10MB, there is something wrong and a producer of such an
> input does not even deserve a nice error message" could be an
> entirely sensible attitude.  But that does not mean an initial
> allocation should be 10MB.  If the expected typical workload fits
> within a lot lower bound, starting from there and allowing it to
> grow up to that maximum would be the more natural thing to do.
>
> And the problem I have with the proposed "fixed" is that it does not
> allow us to do that.