Re: svn commit: r1493272 - /subversion/trunk/subversion/libsvn_client/blame.c

[Johan Corveleyn <jc...@gmail.com>]

On Sat, Jun 15, 2013 at 12:43 AM,  <rh...@apache.org> wrote:
> Author: rhuijben
> Date: Fri Jun 14 22:43:09 2013
> New Revision: 1493272
>
> URL: http://svn.apache.org/r1493272
> Log:
> Revert r1446177, which tweaked the closing time of a blame stream.
>
> This patch caused a huge performance regression in some specific configuration
> triggered by the SharpSvn build.
>
> It also makes it clearer where we stop using the stream to help in applying the
> performance improvements I discussed today with jcorvel around reusing files
> instead of creating a tempfile for every new revision.

Bert,

Can you elaborate a bit? Why did this cause a huge performance
regression in some configurations (but not in others)?

Is this an important problem that might hurt other builds of 1.8.0?
Should this be backported in 1.8.1 or something?

--
Johan

RE: svn commit: r1493272 - /subversion/trunk/subversion/libsvn_client/blame.c

[Bert Huijben <be...@qqmail.nl>]

> -----Original Message-----
> From: Johan Corveleyn [mailto:jcorvel@gmail.com]
> Sent: zondag 16 juni 2013 21:30
> To: Subversion Development
> Cc: Bert Huijben
> Subject: Re: svn commit: r1493272 -
> /subversion/trunk/subversion/libsvn_client/blame.c
> 
> On Sat, Jun 15, 2013 at 12:43 AM,  <rh...@apache.org> wrote:
> > Author: rhuijben
> > Date: Fri Jun 14 22:43:09 2013
> > New Revision: 1493272
> >
> > URL: http://svn.apache.org/r1493272
> > Log:
> > Revert r1446177, which tweaked the closing time of a blame stream.
> >
> > This patch caused a huge performance regression in some specific
> configuration
> > triggered by the SharpSvn build.
> >
> > It also makes it clearer where we stop using the stream to help in
applying
> the
> > performance improvements I discussed today with jcorvel around reusing
> files
> > instead of creating a tempfile for every new revision.
> 
> Bert,
> 
> Can you elaborate a bit? Why did this cause a huge performance
> regression in some configurations (but not in others)?
> 
> Is this an important problem that might hurt other builds of 1.8.0?
> Should this be backported in 1.8.1 or something?

Without this patch we delete a file that has still handles open to it. 
It depends on the apr code (and other concurrent open applications such as
virusscanners) how this behaves and in general this is not recommended.
In my case we tried to delete every tempfile for a few seconds and then
ignored that the tempfile (in %TEMP%) wasn't deleted.

So every blame revision took seconds, and I kept a lot of files in my
tempdir.

	Bert