Hi,
After doing some research I have the following questions.
Why do we write 'path Vs mergeinfo string hash' to disk? Why not read
fs_node_prop for 'svn:mergeinfo'?
From the comment I could understand that it is to avoid the reparsing
of 'svn:mergeinfo' property string.
But we do reparsing already for each and every path in 'svn:mergeinfo'
in index_path_merge_info.
This transactions/<no>-<id>.txn/'mergeinfo hash file just reduces path
parsing not the rev parsing which is normally going to be very big as we
keep doing the merges.
Offtopic:
Anyway observed the svn_fs_t* as seen from commit_body and
fs_change_node_prop are one and the same.
So introduced the 'apr_hash_t *minfo' to svn_fs_t's fsap_data, it
worked! Except for one segfault in merge_tests.py 16th TestCase.
Not sure whether this step is correct or now.
With regards
Kamesh Jayachandran
Daniel Berlin wrote:
> 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 Fri Aug 25 12:37:38 2006