Re: svn commit: r1517733 - in /subversion/trunk/subversion: include/private/svn_file.h libsvn_subr/file.c

[Stefan Fuhrmann <st...@wandisco.com>]

On Mon, Jul 7, 2014 at 4:59 PM, Branko Čibej <br...@wandisco.com> wrote:

> On 07.07.2014 16:25, Ivan Zhakov wrote:
> > On 27 August 2013 03:53,  <st...@apache.org> wrote:
> >> Author: stefan2
> >> Date: Mon Aug 26 23:53:19 2013
> >> New Revision: 1517733
> >>
> >> URL: http://svn.apache.org/r1517733
> >> Log:
> >> Following up on a suggestion by Ivan on @dev about a month ago.
> >>
> >> This is an initial draft plus completely untested implementation
> >> for an SVN file API.  Its implementation works around a few
> >> inefficiencies and scalability issues with apr_file_t.  See the
> >> header comments for details.
> >>
> >> Tests will be added eventually and the code stabilized based on
> >> the test results.  To prevent accidental use in current production
> >> code, the new API is only available if you define #ENABLE_SVN_FILE
> >> in your code.
> >>
> > Hi Stefan,
> >
> > I told you multiple times that I'm against file handling sharing in
> > Subversion code:
> > 1. Operating already maintains shared state files. At least it could
> maintain
> > 2. It very error-prone to maintain shared file handle state in user
> > mode, because file handle may potentially have many hidden state and
> > passing it from different code could create very nasty side effects in
> > multi-threaded applications
> >
> > I'm also think that brain-dumping untested and not ideas to
> > repository: it's very confusing for readers.
> >
> > So could you please revert your change given all reasons above.
> >
> > PS: AFAIK I suggested you to experiment with direct OS file API usage
> > instead of APR if you think that apr_file_t is performance bottleneck.
>
>
> I have to agree with Ivan here. If there is a problem with the
> apr_file_t implementation, then the place to fix it is in APR, not in
> Subversion.
>

I moved the code from /trunk to a dev branch now.

As of now, I'm not entirely sure what to do with it.
Maybe, turning it into something mmap-based for
repository-only (an possibly read-only) use.

-- Stefan^2.