You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Eric Gillespie <ep...@pretzelnet.org> on 2007/04/16 20:38:48 UTC
[PATCH] Don't waste some APR file buffers
In the "Is mod_dav_svn safe for use in a threaded MPM?" thread,
Malcolm Rowe <ma...@farside.org.uk> writes:
> Opening the file in buffered mode when you're going to read
> everything at once just wastes a 4k APR file buffer.
So i looked at the other APR_BUFFERED uses in fsfs, and found
some that were similarly wasteful.
[[[
Don't waste a APR file buffer when reading some trivial files.
* subversion/libsvn_fs_fs/fs_fs.c
(svn_fs_fs__open): Drop APR_BUFFERED when opening UUID file.
(read_current): Drop APR_BUFFERED option when opening current file.
(read_next_ids): Drop APR_BUFFERED option when opening next-ids file.
]]]
Index: fs_fs.c
===================================================================
--- fs_fs.c (revision 24576)
+++ fs_fs.c (working copy)
@@ -848,7 +848,7 @@
/* Read in and cache the repository uuid. */
SVN_ERR(svn_io_file_open(&uuid_file, path_uuid(fs, pool),
- APR_READ | APR_BUFFERED,
APR_OS_DEFAULT, pool));
+ APR_READ, APR_OS_DEFAULT, pool));
limit = sizeof(buf);
SVN_ERR(svn_io_read_length_line(uuid_file, buf, &limit,
pool));
@@ -941,7 +941,7 @@
svn_pool_clear(iterpool);
SVN_RETRY_ESTALE(err, svn_io_file_open(&revision_file,
fname,
- APR_READ |
APR_BUFFERED,
+ APR_READ,
APR_OS_DEFAULT,
iterpool));
len = CURRENT_BUF_LEN;
@@ -3414,7 +3414,7 @@
char *str, *last_str;
SVN_ERR(svn_io_file_open(&file, path_txn_next_ids(fs, txn_id,
pool),
- APR_READ | APR_BUFFERED,
APR_OS_DEFAULT, pool));
+ APR_READ, APR_OS_DEFAULT, pool));
limit = sizeof(buf);
SVN_ERR(svn_io_read_length_line(file, buf, &limit, pool));
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Don't waste some APR file buffers
Posted by Malcolm Rowe <ma...@farside.org.uk>.
On Mon, Apr 16, 2007 at 04:40:43PM -0700, Daniel Rall wrote:
> On Mon, 16 Apr 2007, Eric Gillespie wrote:
>
> > Daniel Rall <dl...@collab.net> writes:
> >
> > > +1, this patch does what's intended. We're sure there's no unexpected
> > > side-effects from this?
> >
> > We're sure it passes the tests. I obviously can't say with
> > certainty that there are no unexpected side effects, but all
> > we're doing is dropping the buffer, which is just an optimization
> > to reduce the number of reads and writes. It's actually a
> > de-optimization (mediocritization? :) for files as tiny as these.
>
> Right. Commit it.
-1, sorry. Check the implementation of svn_io_read_length_line() - it
reads the file byte-by-byte.
Regards,
Malcolm
Re: [PATCH] Don't waste some APR file buffers
Posted by Daniel Rall <dl...@collab.net>.
On Mon, 16 Apr 2007, Eric Gillespie wrote:
> Daniel Rall <dl...@collab.net> writes:
>
> > +1, this patch does what's intended. We're sure there's no unexpected
> > side-effects from this?
>
> We're sure it passes the tests. I obviously can't say with
> certainty that there are no unexpected side effects, but all
> we're doing is dropping the buffer, which is just an optimization
> to reduce the number of reads and writes. It's actually a
> de-optimization (mediocritization? :) for files as tiny as these.
Right. Commit it.
Re: [PATCH] Don't waste some APR file buffers
Posted by Eric Gillespie <ep...@pretzelnet.org>.
Daniel Rall <dl...@collab.net> writes:
> +1, this patch does what's intended. We're sure there's no unexpected
> side-effects from this?
We're sure it passes the tests. I obviously can't say with
certainty that there are no unexpected side effects, but all
we're doing is dropping the buffer, which is just an optimization
to reduce the number of reads and writes. It's actually a
de-optimization (mediocritization? :) for files as tiny as these.
--
Eric Gillespie <*> epg@pretzelnet.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Don't waste some APR file buffers
Posted by Daniel Rall <dl...@collab.net>.
On Mon, 16 Apr 2007, Eric Gillespie wrote:
> In the "Is mod_dav_svn safe for use in a threaded MPM?" thread,
> Malcolm Rowe <ma...@farside.org.uk> writes:
>
> > Opening the file in buffered mode when you're going to read
> > everything at once just wastes a 4k APR file buffer.
>
> So i looked at the other APR_BUFFERED uses in fsfs, and found
> some that were similarly wasteful.
+1, this patch does what's intended. We're sure there's no unexpected
side-effects from this?
> [[[
> Don't waste a APR file buffer when reading some trivial files.
>
> * subversion/libsvn_fs_fs/fs_fs.c
> (svn_fs_fs__open): Drop APR_BUFFERED when opening UUID file.
> (read_current): Drop APR_BUFFERED option when opening current file.
> (read_next_ids): Drop APR_BUFFERED option when opening next-ids file.
> ]]]
>
> Index: fs_fs.c
> ===================================================================
> --- fs_fs.c (revision 24576)
> +++ fs_fs.c (working copy)
> @@ -848,7 +848,7 @@
>
> /* Read in and cache the repository uuid. */
> SVN_ERR(svn_io_file_open(&uuid_file, path_uuid(fs, pool),
> - APR_READ | APR_BUFFERED,
> APR_OS_DEFAULT, pool));
> + APR_READ, APR_OS_DEFAULT, pool));
>
> limit = sizeof(buf);
> SVN_ERR(svn_io_read_length_line(uuid_file, buf, &limit,
> pool));
> @@ -941,7 +941,7 @@
> svn_pool_clear(iterpool);
>
> SVN_RETRY_ESTALE(err, svn_io_file_open(&revision_file,
> fname,
> - APR_READ |
> APR_BUFFERED,
> + APR_READ,
> APR_OS_DEFAULT,
> iterpool));
>
> len = CURRENT_BUF_LEN;
> @@ -3414,7 +3414,7 @@
> char *str, *last_str;
>
> SVN_ERR(svn_io_file_open(&file, path_txn_next_ids(fs, txn_id,
> pool),
> - APR_READ | APR_BUFFERED,
> APR_OS_DEFAULT, pool));
> + APR_READ, APR_OS_DEFAULT, pool));
>
> limit = sizeof(buf);
> SVN_ERR(svn_io_read_length_line(file, buf, &limit, pool));
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org