The FSFS changes cache returns an array representation of changed
path data which needs to be converted back to a hash for API reasons
by svn_fs_fs__paths_changed(). No other function accesses the
changes cache so returning a temporary array seems odd.
Wouldn't it make sense for the cache to return hashes directly?
Isn't it better to convert from hash to array during serialization,
which ideally happens only once, rather than constructing an array
and then a hash from array elements every time the cache is accessed?
I'm not sure how to measure the effects of this.
Index: subversion/libsvn_fs_fs/cached_data.c
===================================================================
--- subversion/libsvn_fs_fs/cached_data.c (revision 1604933)
+++ subversion/libsvn_fs_fs/cached_data.c (working copy)
@@ -2686,7 +2686,7 @@ svn_fs_fs__get_proplist(apr_hash_t **proplist_p,
}
svn_error_t *
-svn_fs_fs__get_changes(apr_array_header_t **changes,
+svn_fs_fs__get_changes(apr_hash_t **changes,
svn_fs_t *fs,
svn_revnum_t rev,
apr_pool_t *result_pool)
@@ -3093,7 +3093,7 @@ auto_select_stream(svn_stream_t **stream,
* FILE and wrapping FILE_STREAM. Use POOL for allocations.
*/
static svn_error_t *
-block_read_changes(apr_array_header_t **changes,
+block_read_changes(apr_hash_t **changes,
svn_fs_t *fs,
svn_fs_fs__revision_file_t *rev_file,
svn_fs_fs__p2l_entry_t *entry,
@@ -3303,7 +3303,7 @@ block_read(void **result,
break;
case SVN_FS_FS__ITEM_TYPE_CHANGES:
- SVN_ERR(block_read_changes((apr_array_header_t **)&item,
+ SVN_ERR(block_read_changes((apr_hash_t **)&item,
fs, revision_file,
entry, is_result,
pool, iterpool));
Index: subversion/libsvn_fs_fs/cached_data.h
===================================================================
--- subversion/libsvn_fs_fs/cached_data.h (revision 1604933)
+++ subversion/libsvn_fs_fs/cached_data.h (working copy)
@@ -150,7 +150,7 @@ svn_fs_fs__get_proplist(apr_hash_t **proplist,
* Allocate the result in POOL.
*/
svn_error_t *
-svn_fs_fs__get_changes(apr_array_header_t **changes,
+svn_fs_fs__get_changes(apr_hash_t **changes,
svn_fs_t *fs,
svn_revnum_t rev,
apr_pool_t *pool);
Index: subversion/libsvn_fs_fs/low_level.c
===================================================================
--- subversion/libsvn_fs_fs/low_level.c (revision 1604933)
+++ subversion/libsvn_fs_fs/low_level.c (working copy)
@@ -381,7 +381,7 @@ read_change(change_t **change_p,
}
svn_error_t *
-svn_fs_fs__read_changes(apr_array_header_t **changes,
+svn_fs_fs__read_changes(apr_hash_t **changes,
svn_stream_t *stream,
apr_pool_t *result_pool,
apr_pool_t *scratch_pool)
@@ -389,15 +389,14 @@ svn_error_t *
change_t *change;
apr_pool_t *iterpool;
- /* pre-allocate enough room for most change lists
- (will be auto-expanded as necessary) */
- *changes = apr_array_make(result_pool, 30, sizeof(change_t *));
+ *changes = apr_hash_make(result_pool);
SVN_ERR(read_change(&change, stream, result_pool, scratch_pool));
iterpool = svn_pool_create(scratch_pool);
while (change)
{
- APR_ARRAY_PUSH(*changes, change_t*) = change;
+ apr_hash_set(*changes, change->path.data, change->path.len,
+ &change->info);
SVN_ERR(read_change(&change, stream, result_pool, iterpool));
svn_pool_clear(iterpool);
}
Index: subversion/libsvn_fs_fs/low_level.h
===================================================================
--- subversion/libsvn_fs_fs/low_level.h (revision 1604933)
+++ subversion/libsvn_fs_fs/low_level.h (working copy)
@@ -86,7 +86,7 @@ svn_fs_fs__unparse_footer(apr_off_t l2p_offset,
/* Read all the changes from STREAM and store them in *CHANGES,
allocated in RESULT_POOL. Do temporary allocations in SCRATCH_POOL. */
svn_error_t *
-svn_fs_fs__read_changes(apr_array_header_t **changes,
+svn_fs_fs__read_changes(apr_hash_t **changes,
svn_stream_t *stream,
apr_pool_t *result_pool,
apr_pool_t *scratch_pool);
Index: subversion/libsvn_fs_fs/temp_serializer.c
===================================================================
--- subversion/libsvn_fs_fs/temp_serializer.c (revision 1604933)
+++ subversion/libsvn_fs_fs/temp_serializer.c (working copy)
@@ -1125,7 +1125,7 @@ deserialize_change(void *buffer, change_t **change
}
/* Auxiliary structure representing the content of a change_t array.
- This structure is much easier to (de-)serialize than an APR array.
+ This structure is much easier to (de-)serialize than an APR hash.
*/
typedef struct changes_data_t
{
@@ -1142,20 +1142,30 @@ svn_fs_fs__serialize_changes(void **data,
void *in,
apr_pool_t *pool)
{
- apr_array_header_t *array = in;
+ apr_hash_t *hash = in;
changes_data_t changes;
svn_temp_serializer__context_t *context;
svn_stringbuf_t *serialized;
int i;
+ apr_hash_index_t *hi;
/* initialize our auxiliary data structure */
- changes.count = array->nelts;
+ changes.count = apr_hash_count(hash);
changes.changes = apr_palloc(pool, sizeof(change_t*) * changes.count);
- /* populate it with the array elements */
- for (i = 0; i < changes.count; ++i)
- changes.changes[i] = APR_ARRAY_IDX(array, i, change_t*);
+ /* populate it with the hash elements */
+ i = 0;
+ for (hi = apr_hash_first(pool, hash); hi; hi = apr_hash_next(hi), ++i)
+ {
+ const char *path = svn__apr_hash_index_key(hi);
+ svn_fs_path_change2_t *info = svn__apr_hash_index_val(hi);
+ changes.changes[i] = apr_palloc(pool, sizeof (change_t));
+ changes.changes[i]->path.data = path;
+ changes.changes[i]->path.len = strlen(path);
+ changes.changes[i]->info = *info;
+ }
+
/* serialize it and all its elements */
context = svn_temp_serializer__init(&changes,
sizeof(changes),
@@ -1188,8 +1198,7 @@ svn_fs_fs__deserialize_changes(void **out,
{
int i;
changes_data_t *changes = (changes_data_t *)data;
- apr_array_header_t *array = apr_array_make(pool, changes->count,
- sizeof(change_t *));
+ apr_hash_t *hash = apr_hash_make(pool);
/* de-serialize our auxiliary data structure */
svn_temp_deserializer__resolve(changes, (void**)&changes->changes);
@@ -1197,13 +1206,17 @@ svn_fs_fs__deserialize_changes(void **out,
/* de-serialize each entry and add it to the array */
for (i = 0; i < changes->count; ++i)
{
+ change_t *change;
+
deserialize_change(changes->changes,
(change_t **)&changes->changes[i]);
- APR_ARRAY_PUSH(array, change_t *) = changes->changes[i];
+ change = changes->changes[i];
+ apr_hash_set(hash, change->path.data, change->path.len,
+ &change->info);
}
/* done */
- *out = array;
+ *out = hash;
return SVN_NO_ERROR;
}
Index: subversion/libsvn_fs_fs/transaction.c
===================================================================
--- subversion/libsvn_fs_fs/transaction.c (revision 1604933)
+++ subversion/libsvn_fs_fs/transaction.c (working copy)
@@ -881,23 +881,8 @@ svn_fs_fs__paths_changed(apr_hash_t **changed_path
svn_revnum_t rev,
apr_pool_t *pool)
{
- apr_hash_t *changed_paths;
- apr_array_header_t *changes;
- int i;
-
- SVN_ERR(svn_fs_fs__get_changes(&changes, fs, rev, pool));
-
- changed_paths = svn_hash__make(pool);
- for (i = 0; i < changes->nelts; ++i)
- {
- change_t *change = APR_ARRAY_IDX(changes, i, change_t *);
- apr_hash_set(changed_paths, change->path.data, change->path.len,
- &change->info);
- }
-
- *changed_paths_p = changed_paths;
-
- return SVN_NO_ERROR;
+ return svn_error_trace(svn_fs_fs__get_changes(changed_paths_p, fs, rev,
+ pool));
}
/* Copy a revision node-rev SRC into the current transaction TXN_ID in
Received on 2014-06-23 23:42:08 CEST