On 7 October 2015 at 15:21, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
> On 7 October 2015 at 14:53, Julian Foad <julianfoad_at_gmail.com> wrote:
>> Ivan Zhakov wrote:
>>> Bert Huijben wrote:
>>>>> URL: http://svn.apache.org/viewvc?rev=1707196&view=rev
>>>>> Log:
>>>>> Slightly optimize svn_stringbuf_from_stream() to avoid allocating twice
>>>>> more memory and unnecessary memcpy() when LEN_HINT is equal to final
>>>>> stringbuf
>>>>> length.
>>>>>
>>>>> * subversion/libsvn_subr/stream.c
>>>>> (svn_stringbuf_from_stream): Always preallocate LEN_HINT +
>>>>> MIN_READ_SIZE
>>>>> bytes to be able perform final read without stringbuf reallocation.
>>>>
>>>> Can you explain why hint + MIN_READ_SIZE instead of something like
>>>> MAX(len_hint+1, MIN_READ_SIZE)
>>>>
>>>> I don't know what MIN_READ_SIZE is from just this patch, but it could easily be
>>>> something like 16 Kbyte, while len_hint could be something like 16 for a file like
>>>> 'format' that was just statted to obtain the actual size
>>>>
>>>> 16 Kbyte + 16 bytes for a 16 bytes file looks a bit large... And allocating 16 + 32
>>>> bytes is far more efficient than allocating that huge chunk at once.
>>>>
>>> MIN_READ_SIZE is 64 bytes and it's locally defined in this function.
>>> We cannot use MAX(len_hint+1, MIN_READ_SIZE) because main loop
>>> reallocates buffer if don't have MIN_READ_SIZE remaining. The current
>>> code assumes that MIN_READ_SIZE is small value and I decided to leave
>>> this code intact. It could be improved of course, but I wanted to make
>>> minimum change first.
>>
>> I don't think that was the right fix. One deficiency with the code is
>> that it doesn't break out of the loop when end-of-stream is detected,
>> which can be reported by a 'short' read.
> It's a different problem and I my plan was to fix in another commit. I
> don't think that problem in existing code should be reason to do not
> fix one of them.
>
Here is the patch that I wanted commit later. What do you think?
--
Ivan Zhakov
Received on 2015-10-07 14:36:08 CEST