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

Re: [PATCH][merge-tracking]diskfile merge info hash to in memory mergeinfo.

From: Daniel Berlin <dberlin_at_spork.dreamhost.com>
Date: 2006-08-23 17:37:57 CEST

This patch won't work, it was the original approach i took.
The problem is that you can reopen existing transactions, and
thus, you have to serialize the data to disk in case it is reopened.

One of the fs regressions tests tests this "feature" (though it doesn't
check the mergeinfo stays with it).

IMHO, this is an FS API wart. Being able to reopen transactions is
"nice", but the fact that it makes us serialize everything to disk is very
bad for performance, and not worth the cost, IMHO.

On Wed, 23 Aug 2006, Kamesh Jayachandran wrote:

> Sorry, There seems to be 2 transactions involved(Not sure why?). This patch
> is ineffective.
>
> Please do not apply.
>
> With regards
> Kamesh Jayachandran
> Kamesh Jayachandran wrote:
>> Hi All,
>> Find the attached patch.
>>
>> 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.
>> Ran the merge-tests.py and found the same functioal behaviour before and
>> after
>> this patch, but slight speed improvements probably due to in memory
>> operations.
>> Disk based merge info hash(merge-tests.py)
>> --------------------------
>> real 3m45.178s
>> user 0m52.591s
>> sys 0m19.689s
>>
>> In memory merge info hash(merge-tests.py)
>> --------------------------
>> real 3m38.203s
>> user 0m52.849s
>> sys 0m19.782s
>>
>> * 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 transaction.
>> (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_txn_data_t):
>> Adding mergeinfo hash 'minfo'.
>> ]]]
>>
>> ------------------------------------------------------------------------
>>
>> Index: subversion/libsvn_fs_fs/fs_fs.c
>> ===================================================================
>> --- subversion/libsvn_fs_fs/fs_fs.c (revision 20955)
>> +++ 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 *
>> @@ -2662,23 +2629,12 @@
>> apr_pool_t *pool)
>> {
>> apr_file_t *txn_minfo_file;
>> - apr_hash_t *txn_minfo = apr_hash_make(pool);
>> + fs_txn_data_t *ftd = txn->fsap_data;
>> + if (ftd->minfo == NULL)
>> + ftd->minfo = apr_hash_make(pool);
>> - SVN_ERR(get_txn_mergeinfo(txn_minfo, txn->fs, txn->id, pool));
>> + apr_hash_set(ftd->minfo, name, APR_HASH_KEY_STRING, value);
>> - 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 +4527,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_txn_data_t *ftd = txn->fsap_data;
>> + *table_p = ftd->minfo;
>> return SVN_NO_ERROR;
>> }
>> Index: subversion/libsvn_fs_fs/fs.h
>> ===================================================================
>> --- subversion/libsvn_fs_fs/fs.h (revision 20955)
>> +++ subversion/libsvn_fs_fs/fs.h (working copy)
>> @@ -77,6 +77,8 @@
>> {
>> /* Merge tracking database. */
>> sqlite3 *mtd;
>> + /* mergeinfo hash. */
>> + apr_hash_t *minfo;
>> } fs_txn_data_t;
>> /* Return a canonicalized version of a filesystem PATH, allocated in
>>
>> ------------------------------------------------------------------------
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
>> For additional commands, e-mail: dev-help@subversion.tigris.org
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Aug 23 17:38:53 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.