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

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.

Bert

-----Original Message-----
From: "Ivan Zhakov" <ivan_at_visualsvn.com>
Sent: ‎7-‎1-‎2014 13:55
To: "Bert Huijben" <bert_at_qqmail.nl>
Cc: "Subversion Development" <dev_at_subversion.apache.org>
Subject: Re: svn commit: r1555716 -/subversion/trunk/subversion/libsvn_ra_serf/update.c

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've tested following
patch to update subversion 1.8.x working copy:
[[[
Index: subversion/libsvn_ra_serf/update.c
===================================================================
--- subversion/libsvn_ra_serf/update.c (revision 1556193)
+++ subversion/libsvn_ra_serf/update.c (working copy)
@@ -3429,7 +3429,7 @@
   *reporter = &ra_serf_reporter;
   *report_baton = report;

- report->body_sb = svn_spillbuf__create(1024, 131072, report->pool);
+ report->body_sb = svn_spillbuf__create(220, 220, report->pool);

   if (sess->bulk_updates == svn_tristate_true)
     {
]]]

It fails with:
[[[
..\..\..\subversion\svn\update-cmd.c:172,
..\..\..\subversion\libsvn_client\update.c:722,
..\..\..\subversion\libsvn_client\update.c:614,
..\..\..\subversion\libsvn_client\update.c:466,
..\..\..\subversion\libsvn_wc\adm_crawler.c:844,
..\..\..\subversion\libsvn_ra_serf\update.c:3131,
..\..\..\subversion\libsvn_ra_serf\util.c:915,
..\..\..\subversion\libsvn_ra_serf\util.c:2270,
..\..\..\subversion\libsvn_ra_serf\util.c:2043,
..\..\..\subversion\libsvn_ra_serf\util.c:2030:
(apr_err=SVN_ERR_RA_DAV_REQUEST_FAILED)
svn: E175002: Unexpected HTTP status 400 'Bad Request' on '/repos/asf/!svn/me'
]]]

> If you want to look at improving it now, feel free. I'm first working on
> switching the update logic to the new xml parser. (And will perform a
> further cleanup including these points round after that).
>
I suggest to remove this optimized code path completely: potential
optimization is not significant imho. Also such big update request
bodies are not usual. I can remove this code if you're busy with other
stuff.

-- 
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com