You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2011/12/18 13:56:16 UTC
svn commit: r1220388 - in /subversion/branches/file-handle-cache/subversion:
include/private/svn_file_handle_cache.h libsvn_fs_fs/fs_fs.c
libsvn_subr/svn_file_handle_cache.c
Author: stefan2
Date: Sun Dec 18 12:56:16 2011
New Revision: 1220388
URL: http://svn.apache.org/viewvc?rev=1220388&view=rev
Log:
On file_handle_cache branch: Simplify the file handle cache API.
No longer require cookies, perms and flags. The first will be replaced by a smarter
internal file handle selection logic and the latter will be hard-coded since this cache
is only useful for files with buffered read.
* subversion/include/private/svn_file_handle_cache.h
(svn_file_handle_cache__open): drop parameters from API declaration
* subversion/libsvn_subr/svn_file_handle_cache.c
adapt global commentary
(cache_entry_t): remove now unused element from cache entry
(internal_file_open): drop the same 3 parameters
(are_siblings): new utility function
(svn_file_handle_cache__open_internal): prefer opening a small number
of file handles for the same file over moving file pointers back and forth
and reading the same data many times.
(svn_file_handle_cache__open): adapt pass-through function
* subversion/libsvn_fs_fs/fs_fs.c
(DEFAULT_FILE_COOKIE, REP_FILE_COOKIE): no longer needed
(open_pack_or_rev_file, open_and_seek_revision, open_and_seek_transaction):
remove cookie from signature; adapt API and function calls
(get_node_revision_body, svn_fs_fs__rev_get_root, svn_fs_fs__paths_changed,
recover_get_largest_revision, recover_body): adapt API and function calls
Modified:
subversion/branches/file-handle-cache/subversion/include/private/svn_file_handle_cache.h
subversion/branches/file-handle-cache/subversion/libsvn_fs_fs/fs_fs.c
subversion/branches/file-handle-cache/subversion/libsvn_subr/svn_file_handle_cache.c
Modified: subversion/branches/file-handle-cache/subversion/include/private/svn_file_handle_cache.h
URL: http://svn.apache.org/viewvc/subversion/branches/file-handle-cache/subversion/include/private/svn_file_handle_cache.h?rev=1220388&r1=1220387&r2=1220388&view=diff
==============================================================================
--- subversion/branches/file-handle-cache/subversion/include/private/svn_file_handle_cache.h (original)
+++ subversion/branches/file-handle-cache/subversion/include/private/svn_file_handle_cache.h Sun Dec 18 12:56:16 2011
@@ -53,24 +53,15 @@ typedef
struct svn_file_handle_cache__handle_t svn_file_handle_cache__handle_t;
/**
- * Get an open file handle in @a f, for the file named @a fname with the
- * open flag(s) in @a flag and permissions in @a perm. These parameters
- * are the same as in @ref svn_io_file_open. The file pointer will be
- * moved to the specified @a offset, if it is different from -1.
- *
- * If there are one or more unused matching open file handles, those with
- * the specified @a cookie will be preferred. This is particularly useful
- * if @a offset is -1, i.e. if the file pointer position of the handle
- * returned is undefined.
+ * Get an open buffered read file handle in @a f, for the file named
+ * @a fname. The file pointer will be moved to the specified @a offset,
+ * if it is different from -1.
*/
svn_error_t *
svn_file_handle_cache__open(svn_file_handle_cache__handle_t **f,
svn_file_handle_cache_t *cache,
const char *fname,
- apr_int32_t flag,
- apr_fileperms_t perm,
apr_off_t offset,
- int cookie,
apr_pool_t *pool);
/**
Modified: subversion/branches/file-handle-cache/subversion/libsvn_fs_fs/fs_fs.c
URL: http://svn.apache.org/viewvc/subversion/branches/file-handle-cache/subversion/libsvn_fs_fs/fs_fs.c?rev=1220388&r1=1220387&r2=1220388&view=diff
==============================================================================
--- subversion/branches/file-handle-cache/subversion/libsvn_fs_fs/fs_fs.c (original)
+++ subversion/branches/file-handle-cache/subversion/libsvn_fs_fs/fs_fs.c Sun Dec 18 12:56:16 2011
@@ -114,17 +114,6 @@
#define REP_PLAIN "PLAIN"
#define REP_DELTA "DELTA"
-/* Cookies used to classify cached file handle usage */
-/* Used whenever no other specific region of the rev file is being read. */
-#define DEFAULT_FILE_COOKIE 0
-
-/* Used when reading representation data.
- * Since this is often interleaved with other reads, use a separate
- * cookie (hence a separate file handle) for the reps. That way, rep
- * access can often be satisfied from the APR read buffer. The same
- * applies to the meta data because it is not rep data. */
-#define REP_FILE_COOKIE 1
-
/* Notes:
To avoid opening and closing the rev-files all the time, it would
@@ -1792,8 +1781,7 @@ ensure_revision_exists(svn_fs_t *fs,
been packed, *FILE will be set to the packed file; otherwise, set *FILE
to the revision file for REV. Return SVN_ERR_FS_NO_SUCH_REVISION if the
file doesn't exist. Move the file pointer of OFFSET, if the latter is
- not -1. Prefer cached file handles that share the same COOKIE (again,
- if not -1).
+ not -1.
TODO: Consider returning an indication of whether this is a packed rev
file, so the caller need not rely on is_packed_rev() which in turn
@@ -1806,7 +1794,6 @@ open_pack_or_rev_file(svn_file_handle_ca
svn_fs_t *fs,
svn_revnum_t rev,
apr_off_t offset,
- int cookie,
apr_pool_t *pool)
{
fs_fs_data_t *ffd = fs->fsap_data;
@@ -1823,10 +1810,7 @@ open_pack_or_rev_file(svn_file_handle_ca
err = svn_file_handle_cache__open(file,
ffd->file_handle_cache,
path,
- APR_READ | APR_BUFFERED,
- APR_OS_DEFAULT,
offset,
- cookie,
pool);
if (err && APR_STATUS_IS_ENOENT(err->apr_err)
@@ -1934,7 +1918,6 @@ open_and_seek_revision(svn_file_handle_c
svn_fs_t *fs,
svn_revnum_t rev,
apr_off_t offset,
- int cookie,
apr_pool_t *pool)
{
/* none of the following requires the file handle */
@@ -1948,7 +1931,7 @@ open_and_seek_revision(svn_file_handle_c
}
/* So, open the revision file and position the pointer here in one go. */
- return open_pack_or_rev_file(file, fs, rev, offset, cookie, pool);
+ return open_pack_or_rev_file(file, fs, rev, offset, pool);
}
/* Open the representation for a node-revision in transaction TXN_ID
@@ -1962,7 +1945,6 @@ open_and_seek_transaction(svn_file_handl
svn_fs_t *fs,
const char *txn_id,
representation_t *rep,
- int cookie,
apr_pool_t *pool)
{
fs_fs_data_t *ffd = fs->fsap_data;
@@ -1971,10 +1953,7 @@ open_and_seek_transaction(svn_file_handl
return svn_file_handle_cache__open(file,
ffd->file_handle_cache,
path_txn_proto_rev(fs, txn_id, pool),
- APR_READ | APR_BUFFERED,
- APR_OS_DEFAULT,
rep->offset,
- cookie,
pool);
}
@@ -1992,10 +1971,9 @@ open_and_seek_representation(svn_file_ha
* buffer effectiveness. */
if (! rep->txn_id)
return open_and_seek_revision(file_p, fs, rep->revision, rep->offset,
- REP_FILE_COOKIE, pool);
+ pool);
else
- return open_and_seek_transaction(file_p, fs, rep->txn_id, rep,
- REP_FILE_COOKIE, pool);
+ return open_and_seek_transaction(file_p, fs, rep->txn_id, rep, pool);
}
/* Parse the description of a representation from STRING and store it
@@ -2218,10 +2196,7 @@ get_node_revision_body(node_revision_t *
err = svn_file_handle_cache__open(&revision_file,
ffd->file_handle_cache,
path_txn_node_rev(fs, id, pool),
- APR_READ | APR_BUFFERED,
- APR_OS_DEFAULT,
0,
- DEFAULT_FILE_COOKIE,
pool);
}
else
@@ -2230,7 +2205,6 @@ get_node_revision_body(node_revision_t *
err = open_and_seek_revision(&revision_file, fs,
svn_fs_fs__id_rev(id),
svn_fs_fs__id_offset(id),
- DEFAULT_FILE_COOKIE,
pool);
}
@@ -2928,8 +2902,7 @@ svn_fs_fs__rev_get_root(svn_fs_id_t **ro
return SVN_NO_ERROR;
/* we don't care about the file pointer position */
- SVN_ERR(open_pack_or_rev_file(&revision_file, fs, rev, -1,
- DEFAULT_FILE_COOKIE, pool));
+ SVN_ERR(open_pack_or_rev_file(&revision_file, fs, rev, -1,pool));
apr_rev_file = svn_file_handle_cache__get_apr_handle(revision_file);
/* here, it will get moved anyways */
@@ -4681,8 +4654,7 @@ svn_fs_fs__paths_changed(apr_hash_t **ch
SVN_ERR(ensure_revision_exists(fs, rev, pool));
/* we don't care about the file pointer position */
- SVN_ERR(open_pack_or_rev_file(&revision_file, fs, rev, -1,
- DEFAULT_FILE_COOKIE, pool));
+ SVN_ERR(open_pack_or_rev_file(&revision_file, fs, rev, -1, pool));
apr_revision_file = svn_file_handle_cache__get_apr_handle(revision_file);
/* here, it will get moved anyways */
@@ -6778,8 +6750,7 @@ recover_get_largest_revision(svn_fs_t *f
/* We don't care about the file pointer position as long as the file
itself exists. */
- err = open_pack_or_rev_file(&file, fs, right, -1,
- DEFAULT_FILE_COOKIE, iterpool);
+ err = open_pack_or_rev_file(&file, fs, right, -1, iterpool);
svn_pool_clear(iterpool);
if (err && err->apr_err == SVN_ERR_FS_NO_SUCH_REVISION)
@@ -6804,8 +6775,7 @@ recover_get_largest_revision(svn_fs_t *f
svn_file_handle_cache__handle_t *file;
/* Again, ignore the file pointer position. */
- err = open_pack_or_rev_file(&file, fs, probe, -1,
- DEFAULT_FILE_COOKIE, iterpool);
+ err = open_pack_or_rev_file(&file, fs, probe, -1, iterpool);
svn_pool_clear(iterpool);
if (err && err->apr_err == SVN_ERR_FS_NO_SUCH_REVISION)
@@ -7095,8 +7065,7 @@ recover_body(void *baton, apr_pool_t *po
SVN_ERR(b->cancel_func(b->cancel_baton));
/* Any file pointer position will do ... */
- SVN_ERR(open_pack_or_rev_file(&rev_file, fs, rev, -1,
- DEFAULT_FILE_COOKIE, iterpool));
+ SVN_ERR(open_pack_or_rev_file(&rev_file, fs, rev, -1, iterpool));
apr_rev_file = svn_file_handle_cache__get_apr_handle(rev_file);
/* ... because it gets set here explicitly */
Modified: subversion/branches/file-handle-cache/subversion/libsvn_subr/svn_file_handle_cache.c
URL: http://svn.apache.org/viewvc/subversion/branches/file-handle-cache/subversion/libsvn_subr/svn_file_handle_cache.c?rev=1220388&r1=1220387&r2=1220388&view=diff
==============================================================================
--- subversion/branches/file-handle-cache/subversion/libsvn_subr/svn_file_handle_cache.c (original)
+++ subversion/branches/file-handle-cache/subversion/libsvn_subr/svn_file_handle_cache.c Sun Dec 18 12:56:16 2011
@@ -55,8 +55,7 @@
* the current file pointer gets (optionally) compared with the location
* of the next access. If a cached file is found that may already have the
* desired data in its buffer, that handle will be returned instead of a
- * random one. When the position of the next file access is not known,
- * a cookie may be used to discern files in a similar way.
+ * random one.
*
* For read-after-write scenarios, it is imperative to flush the APR file
* buffer before attempting to read that file. Therefore, all idle handles
@@ -117,7 +116,8 @@ struct entry_list_t
* so we can reuse the memory by clearing the pool.
*
* The cache entry is linked in three lists:
- * - the global list of either used or unused entry (having no file handle)
+ * - the global list of either used or unused entries
+ * (unused ones are those having no APR file handle)
* - list of sibblings, i.e. file handles to the same file
* - global LRU list of idle file handles, i.e. those that are currently
* not used by the application
@@ -140,15 +140,6 @@ struct cache_entry_t
/* the file name. NULL for unused entries */
const char *name;
- /* file open flag(s). Valid only for used entries. */
- apr_int32_t flag;
-
- /* granted file permissions. Valid only for used entries. */
- apr_fileperms_t perm;
-
- /* granted file permissions. Valid only for used entries. */
- int cookie;
-
/* position of the file pointer. Valid only for idle entries. */
apr_off_t position;
@@ -356,17 +347,13 @@ auto_close_cached_handle(void *entry_voi
return APR_SUCCESS;
}
-/* Create a new APR-level file handle with the specified file NAME, FLAG
- * and permissions PERM. A COOKIE will be attached to it. The corresponding
- * cache item will be returned in RESULT.
+/* Create a new APR-level file handle with the specified file NAME in CACHE.
+ * The corresponding cache entry will be returned in RESULT.
*/
static svn_error_t *
internal_file_open(cache_entry_t **result,
svn_file_handle_cache_t *cache,
- const char *name,
- apr_int32_t flag,
- apr_fileperms_t perm,
- int cookie)
+ const char *name)
{
cache_entry_t *entry;
cache_entry_t *sibling;
@@ -395,7 +382,8 @@ internal_file_open(cache_entry_t **resul
}
/* (try to) open the requested file */
- SVN_ERR(svn_io_file_open(&entry->file, name, flag, perm, entry->pool));
+ SVN_ERR(svn_io_file_open(&entry->file, name, APR_READ | APR_BUFFERED,
+ APR_OS_DEFAULT, entry->pool));
assert(entry->file);
/* make sure we auto-close cached file handles held by the application
@@ -406,9 +394,6 @@ internal_file_open(cache_entry_t **resul
apr_pool_cleanup_null);
/* set file info */
- entry->flag = flag;
- entry->perm = perm;
- entry->cookie = cookie;
entry->name = apr_pstrdup(entry->pool, name);
entry->position = 0;
@@ -608,6 +593,14 @@ pointer_is_closer(const cache_entry_t *e
return old_delta > new_delta ? TRUE : FALSE;
}
+/* Returns true if LHS and RHS refer to the same file.
+ */
+static svn_boolean_t
+are_siblings(const cache_entry_t *lhs, const cache_entry_t *rhs)
+{
+ return (lhs == rhs) || !strcmp(lhs->name, rhs->name);
+}
+
/* Set file pointer of ENTRY->file to OFFSET. As an optimization, make sure
* that a few hundred bytes before that OFFSET are also pre-fetched as SVN
* tends to read data "backwards".
@@ -635,63 +628,94 @@ aligned_seek(cache_entry_t *entry, apr_o
* flag(s) in FLAG and permissions in PERM. These parameters are the same
* as in svn_io_file_open(). The file pointer will be moved to the specified
* OFFSET, if it is different from -1.
- *
- * If the COOKIE is not -1, only those file handles will be considerd a match
- * that have been opened with that cookie before. This is particularly useful
- * if OFFSET is -1, i.e. if the file pointer position of the handle returned
- * is undefined.
*/
static svn_error_t *
svn_file_handle_cache__open_internal
(svn_file_handle_cache__handle_t **f,
svn_file_handle_cache_t *cache,
const char *fname,
- apr_int32_t flag,
- apr_fileperms_t perm,
apr_off_t offset,
- int cookie,
apr_pool_t *pool)
{
- svn_error_t *err = SVN_NO_ERROR;
cache_entry_t *entry;
- cache_entry_t *next_entry;
+ cache_entry_t *first_entry;
cache_entry_t *near_entry = NULL;
- cache_entry_t *cookie_entry = NULL;
+ cache_entry_t *any_entry = NULL;
+ cache_entry_t *last_entry = NULL;
cache_entry_t *entry_found = NULL;
+
+ int idle_entry_count = 0;
/* look through all idle entries for this filename and find suitable ones */
- for (entry = find_first(cache, fname); entry; entry = next_entry)
+ first_entry = find_first(cache, fname);
+ for ( entry = first_entry
+ ; entry
+ ; entry = get_next_entry (&entry->sibling_link))
{
- next_entry = get_next_entry (&entry->sibling_link);
+ last_entry = entry;
+ assert(entry->file != NULL);
+
if (! entry->open_handle)
{
- /* The entry is idle. Is it suitable? */
- if (entry->flag == flag && entry->perm == perm)
+ idle_entry_count++;
+
+ if (! any_entry)
+ any_entry = entry;
+
+ /* is it a particularly close match? */
+ if (pointer_is_closer(entry, offset, near_entry))
+ near_entry = entry;
+ }
+ }
+
+ /* select the most suitable idle file handle */
+ if (near_entry)
+ {
+ /* best option: a file whose buffer propably already contains
+ * the data that we are looking for. */
+ entry_found = near_entry;
+ }
+ else if (any_entry)
+ {
+ /* Re-using an open file is also a good idea.
+ *
+ * However, it may be better to open packed files a few more times
+ * since later we are likely to read data later close to the current
+ * location. Keep the number open handles / file reasonably low.
+ */
+ if ( (idle_entry_count >= 4)
+ || ( (cache->unused_entries.count == 0)
+ /* auto-closing a suitable file doesn't make sense */
+ && (are_siblings(cache->idle_entries.first->item, any_entry))))
+ {
+ /* Ensure that any_entry will be the last one to be re-used in
+ * successive calls: put it at the end of the siblings list for
+ * this file name.
+ */
+ if (last_entry != any_entry)
{
- /* we can use this entry, if the cookie matches */
- if (entry->cookie == cookie && cookie != -1)
- {
- cookie_entry = entry;
-
- /* is it a particularly close match? */
- if (pointer_is_closer(entry, offset, near_entry))
- near_entry = entry;
+ if (any_entry == first_entry)
+ {
+ first_entry = get_next_entry(&any_entry->sibling_link);
+ assert(first_entry->file != NULL);
+ apr_hash_set(cache->first_by_name,
+ any_entry->name,
+ APR_HASH_KEY_STRING,
+ NULL);
+ apr_hash_set(cache->first_by_name,
+ first_entry->name,
+ APR_HASH_KEY_STRING,
+ first_entry);
}
+
+ unlink_link(&any_entry->sibling_link);
+ link_link(&any_entry->sibling_link, &last_entry->sibling_link);
}
- else
- {
- /* the old file handle has been opened with different flags,
- * e.g. this is "read" after "write" or vice versa. Therefore,
- * close the underlying APR handle.
- */
- internal_close_file(cache, entry);
- }
+
+ entry_found = any_entry;
}
}
-
- /* the best match we found */
- entry_found = near_entry ? near_entry : cookie_entry;
-
+
if (entry_found)
{
/* we can use an idle entry */
@@ -729,20 +753,14 @@ svn_error_t *
svn_file_handle_cache__open(svn_file_handle_cache__handle_t **f,
svn_file_handle_cache_t *cache,
const char *fname,
- apr_int32_t flag,
- apr_fileperms_t perm,
apr_off_t offset,
- int cookie,
apr_pool_t *pool)
{
SVN_MUTEX__WITH_LOCK(cache->mutex,
svn_file_handle_cache__open_internal(f,
cache,
fname,
- flag,
- perm,
offset,
- cookie,
pool));
return SVN_NO_ERROR;