[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

[PATCH] [merge-tracking] in memory mergeinfo hash for fs_fs

From: Kamesh Jayachandran <kamesh_at_collab.net>
Date: 2006-08-30 14:33:59 CEST

Hi All,
Found a way to cache 'mergeinfo hash' across transactions in memory.
If this patch is accepted may be we can do the same for other diskbased
persistence in 'bdb'.

With regards
Kamesh Jayachandran

[[[
Patch by: Kamesh Jayachandran <kamesh@collab.net>

Currently the 'mergeinfo property' hash is kept in a disk file.
For every mergeinfo property in the commit, we open a file
read the existing hash and update the hash and overwrite the hash again.
As this 'mergeinfo' is not shared across processes, this mergeinfo could be
kept in the memory itself rather than the disk.
Ideally this 'mergeinfo' hash table need to be cached at the transaction
level.
Unfortunately this is not possible because of the following,
1) fs_change_node_prop do not recieve 'already open txn' as an argument.
2) No way to get the open txn from the 'svn_fs_root_t*'.
3) So 'fs_change_node_prop' creates its own 'txn' and 'commit_body' recieves
   some other open transaction.
As the transactions are different, caching at the txn level will be
ineffective.

So we are left with caching at the 'fs' level(Even though the txns are
different,
they both share the same 'fs' pointer).
Ran the merge-tests.py and found the same functioal behaviour before and
after
this patch, but good speed improvements probably due to in memory
operations.
Found the proper records in the sqlite database with and without this patch.

single commit (after merge) without the patch
---------------------------------------------
real 0m0.886s
user 0m0.047s
sys 0m0.021s

single commit (after merge) with the patch
---------------------------------------------
real 0m0.282s
user 0m0.046s
sys 0m0.017s

Why I am concerned about this change?
Having the in memory representation will be easy to port the same stuff
to 'bdb',
or else I need to change the 'schema of transactions' table hold this
details,
which I don't like to do.

* subversion/libsvn_fs_fs/fs_fs.c
  (PATH_TXN_MERGEINFO): Removed.
  (path_txn_mergeinfo): Removed.
  (get_txn_mergeinfo): Removed.
  (svn_fs_fs__change_txn_mergeinfo):
   Make use of the 'minfo' hash available as a part of the txn->fs.
  (svn_fs_fs__txn_mergeinfo):
   Make use of the 'minfo' hash available as a part of the transaction.
* subversion/libsvn_fs_fs/fs.h
  (struct fs_fs_data_t):
   Adding mergeinfo hash 'minfo'.
]]]

Index: subversion/libsvn_fs_fs/fs_fs.c
===================================================================
--- subversion/libsvn_fs_fs/fs_fs.c (revision 21263)
+++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
@@ -75,7 +75,6 @@
 #define PATH_TXN_PROPS "props" /* Transaction properties */
 #define PATH_NEXT_IDS "next-ids" /* Next temporary ID assignments */
 #define PATH_REV "rev" /* Proto rev file */
-#define PATH_TXN_MERGEINFO "mergeinfo" /* Transaction mergeinfo props */
 #define PATH_PREFIX_NODE "node." /* Prefix for node filename */
 #define PATH_EXT_TXN ".txn" /* Extension of txn dir */
 #define PATH_EXT_CHILDREN ".children" /* Extension for dir contents */
@@ -200,13 +199,6 @@
 }
 
 static const char *
-path_txn_mergeinfo(svn_fs_t *fs, const char *txn_id, apr_pool_t *pool)
-{
- return svn_path_join(path_txn_dir(fs, txn_id, pool), PATH_TXN_MERGEINFO,
- pool);
-}
-
-static const char *
 path_txn_next_ids(svn_fs_t *fs, const char *txn_id, apr_pool_t *pool)
 {
   return svn_path_join(path_txn_dir(fs, txn_id, pool), PATH_NEXT_IDS, pool);
@@ -2628,31 +2620,6 @@
   return SVN_NO_ERROR;
 }
 
-/* Store the mergeinfo list for transaction TXN_ID in MINFO.
- Perform temporary allocations in POOL. */
-
-static svn_error_t *
-get_txn_mergeinfo(apr_hash_t *minfo,
- svn_fs_t *fs,
- const char *txn_id,
- apr_pool_t *pool)
-{
- apr_file_t *txn_minfo_file;
-
- /* Open the transaction mergeinfo file. */
- SVN_ERR(svn_io_file_open(&txn_minfo_file,
- path_txn_mergeinfo(fs, txn_id, pool),
- APR_READ | APR_CREATE | APR_BUFFERED,
- APR_OS_DEFAULT, pool));
-
- /* Read in the property list. */
- SVN_ERR(svn_hash_read(minfo, txn_minfo_file, pool));
-
- SVN_ERR(svn_io_file_close(txn_minfo_file, pool));
-
- return SVN_NO_ERROR;
-}
-
 /* Change mergeinfo for path NAME in TXN to VALUE. */
 
 svn_error_t *
@@ -2661,24 +2628,15 @@
                                 const svn_string_t *value,
                                 apr_pool_t *pool)
 {
- apr_file_t *txn_minfo_file;
- apr_hash_t *txn_minfo = apr_hash_make(pool);
+ fs_fs_data_t *ffd = txn->fs->fsap_data;
+ const char *name_in_fs_pool = apr_pstrdup(txn->fs->pool, name);
+ const svn_string_t *value_in_fs_pool = svn_string_dup(value, txn->fs->pool);
+ if (ffd->minfo == NULL)
+ ffd->minfo = apr_hash_make(txn->fs->pool);
 
- SVN_ERR(get_txn_mergeinfo(txn_minfo, txn->fs, txn->id, pool));
+ apr_hash_set(ffd->minfo, name_in_fs_pool,
+ APR_HASH_KEY_STRING, value_in_fs_pool);
 
- apr_hash_set(txn_minfo, name, APR_HASH_KEY_STRING, value);
-
- /* Create a new version of the file and write out the new minfos. */
- /* Open the transaction minfoerties file. */
- SVN_ERR(svn_io_file_open(&txn_minfo_file,
- path_txn_mergeinfo(txn->fs, txn->id, pool),
- APR_WRITE | APR_CREATE | APR_TRUNCATE
- | APR_BUFFERED, APR_OS_DEFAULT, pool));
-
- SVN_ERR(svn_hash_write(txn_minfo, txn_minfo_file, pool));
-
- SVN_ERR(svn_io_file_close(txn_minfo_file, pool));
-
   return SVN_NO_ERROR;
 }
 
@@ -4571,10 +4529,8 @@
                          svn_fs_txn_t *txn,
                          apr_pool_t *pool)
 {
- apr_hash_t *mergeinfo = apr_hash_make(pool);
- SVN_ERR(get_txn_mergeinfo(mergeinfo, txn->fs, txn->id, pool));
- *table_p = mergeinfo;
-
+ fs_fs_data_t *ffd = txn->fs->fsap_data;
+ *table_p = ffd->minfo;
   return SVN_NO_ERROR;
 }
 
Index: subversion/libsvn_fs_fs/fs.h
===================================================================
--- subversion/libsvn_fs_fs/fs.h (revision 21263)
+++ subversion/libsvn_fs_fs/fs.h (working copy)
@@ -69,6 +69,9 @@
      uuid; discovered using the serialized_init function. */
   apr_thread_mutex_t *lock;
 #endif
+
+ /* mergeinfo hash. */
+ apr_hash_t *minfo;
 } fs_fs_data_t;
 
 /* Transactions need their own private opened copy of the mergeinfo

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Aug 30 14:34:47 2006

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.