stef...@apache.org wrote on Wed, Feb 08, 2012 at 00:44:26 -0000:
> Author: stefan2
> Date: Wed Feb 8 00:44:26 2012
> New Revision: 1241718
>
> URL: http://svn.apache.org/viewvc?rev=1241718&view=rev
> Log:
> Major improvement in delta window handling: Cache intermediate
> combined delta windows such that changes "close by" won't need
> to discover and read the whole chain again.
>
> For algorithms that traverse history linearly, this optimization
> gives delta combination an amortized constant runtime.
>
> For now, we only cache representations < 100kB. Support for larger
> reps can be added later.
>
> * subversion/libsvn_fs_fs/fs.h
> (fs_fs_data_t): add cache for combined windows
> * subversion/libsvn_fs_fs/caching.c
> (svn_fs_fs__initialize_caches): initialize that cache
>
> * subversion/libsvn_fs_fs/fs_fs.c
> (rep_state): add reference to new cache
> (create_rep_state_body): init that reference
> (rep_read_baton): add reference to cached base window
> (get_cached_combined_window, set_cached_combined_window):
> new utility functions
> (build_rep_list): terminate delta chain early if cached
> base window is available
> (rep_read_get_baton): adapt caller
> (get_combined_window): re-implement
> (get_contents): handle new special case; adapt to
> get_combined_window() signature changes
>
> Modified:
> subversion/trunk/subversion/libsvn_fs_fs/caching.c
> subversion/trunk/subversion/libsvn_fs_fs/fs.h
> subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
>
I haven't reviewed this, but a question:
> +/* Read the WINDOW_P for the rep state RS from the current FSFS session's
> + * cache. This will be a no-op and IS_CACHED will be set to FALSE if no
> + * cache has been given. If a cache is available IS_CACHED will inform
> + * the caller about the success of the lookup. Allocations (of the window
> + * in particualar) will be made from POOL.
> + */
> +static svn_error_t *
> +get_cached_combined_window(svn_stringbuf_t **window_p,
> + struct rep_state *rs,
> + svn_boolean_t *is_cached,
> + apr_pool_t *pool)
> +{
> + if (! rs->combined_cache)
> + {
> + /* txdelta window has not been enabled */
> + *is_cached = FALSE;
> + }
> + else
> + {
> + /* ask the cache for the desired txdelta window */
> + return svn_cache__get((void **)window_p,
> + is_cached,
> + rs->combined_cache,
> + get_window_key(rs, rs->start, pool),
How does the cache key identify the particular combined window being
cached?
get_window_key() may return "". If it returns "" when called as an
argument to svn_cache__set() and then also here, won't the cache return
a wrong result?
> + pool);
> + }
> +
> + return SVN_NO_ERROR;
> +}
> +
> +/* Store the WINDOW read at OFFSET for the rep state RS in the current
> + * FSFS session's cache. This will be a no-op if no cache has been given.
> + * Temporary allocations will be made from SCRATCH_POOL. */
> +static svn_error_t *
> +set_cached_combined_window(svn_stringbuf_t *window,
> + struct rep_state *rs,
> + apr_off_t offset,
> + apr_pool_t *scratch_pool)
> +{
> + if (rs->combined_cache)
> + {
> + /* but key it with the start offset because that is the known state
> + * when we will look it up */
> + return svn_cache__set(rs->combined_cache,
> + get_window_key(rs, offset, scratch_pool),
> + window,
> + scratch_pool);
> + }
> +
> + return SVN_NO_ERROR;
> +}
