[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1555716 -/subversion/trunk/subversion/libsvn_ra_serf/update.c

From: Branko Čibej <brane_at_wandisco.com>
Date: Thu, 09 Jan 2014 13:30:17 +0100

On 09.01.2014 10:38, Ivan Zhakov wrote:
> On 9 January 2014 13:29, Branko Čibej <brane_at_wandisco.com> wrote:
>> On 09.01.2014 10:18, Ivan Zhakov wrote:
>>
>> On 7 January 2014 18:29, Bert Huijben <bert_at_qqmail.nl> wrote:
>>
>> On 7 January 2014 12:42, Bert Huijben <bert_at_qqmail.nl> wrote:
>>
>> The current code works in both scenarios (tested via buffersizes of just a
>> few bytes, and a debugger verifyimg the corner cases, etc). But I doubt it
>> is worth all the trouble. The file is null until something is spilled and
>> from that point onwards everything is in the file (until the spill is
>> empty).
>>
>> Few bytes is bad test because memory isn't used at all in this case.
>>
>> I've tested and code doesn't work as expected. > I used 128, 256 but didn't
>> get a problem. The same values might also hit a
>> corner case in the spill buffer. Will look at it in a few minutes.
>>
>> Hi Bert.
>>
>> I see you already removed this optimization in r1556343. Thanks.
>>
>> But it seems there is another problem with using spillbuf for request
>> bodies: create_update_report_body() callback can be called multiple
>> times if request need to be resend because of authentication challenge
>> or KeepAlive limit. In this case you will get empty request body on
>> second invocation, because all spillbuf is already read.
>>
>> I see two options how to fix this:
>> 1. Do not use spillbuf for request bodies. Use custom implementation
>> based on stringbuf and temporary file if needed.
>> 2. Implement svn_spillbuf__rewind() or something.
>>
>> What do you think?
>>
>>
>> Definitely implement rewind, IMO.
>>
> I'm not sure, because as far I remember orignally spillbuf was
> implemented with queue/circular buffer behavior. But then spillbuf was
> extended for other use cases, so may be there is no problem with
> svn_spillbuf__rewind().

You're right about the original intent. The extensions for other use
cases are not used anywhere at the moment, fwiw; my idea was to use the
underlying common functionality for compressed pristines, but I never
got around to getting any further than that.

-- Brane

-- 
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. brane_at_wandisco.com
Received on 2014-01-09 13:31:05 CET

This is an archived mail posted to the Subversion Dev mailing list.