[PATCH] Store wcprops in one file per .svn area

["Peter N. Lundblad" <pe...@famlundblad.se>]

Hi,

There is one more wc format change I'd like in 1.4.  (This is the last
change, promise!).  The point is to avoid lots of wcprops files which
are typically very small (100 bytes ore something) but still take up
an inode and a disk block.

To avoid O(n^2) complexity when setting wcprops during update/commit,
I cache all wcprops in memory and write once at the end of the log run
like we do with entries.  Switch currently has quadratic behaviour
when invalidating all wcprops, but that's easily fixed.

The patch is against the nonxml-entries branch because that was
simplest for me, but it isn't dependent on that work.

Any objections to the idea or to the patch in itself?

[[[
Use one file per admin area for wcprops instead of one file per entry.

* subversion/libsvn_wc/props.c
  (read_wcprops, svn_wc__wcprops_write): New function.
  (svn_wc__wcprop_list): Rename from wcprop_list().  Try to read wcprops
  into cache if possible.  Change path argument to entryname.
  (wcprop_get): Rename from svn_wc__wcprop_get().  Adjust for new signature
  of svn_wc__wcprop_list().
  (svn_wc__wcprop_set): Add force_write parameter.  Write from the cache
  if the WC format is new enough.
  (svn_wc__remove_wcprops):  MOve here.  Add name parameter to support
  removing all wcprops from a single entry.  Support new format.

* subversion/libsvn_wc/props.h
  (svn_wc__wcprop_get): Remove prototype.
  (svn_wc__wcprop_list): Declare.
  (svn_wc__wcprop_set): Add force_write parameter.  Callrs updated.
  (svn_wc__remove_wcprops): Add name parameter.  Callrs updated.
  (svn_wc__wcprops_write): Declare.

* subversion/libsvn_wc/copy.c (svn_wc__remove_wcprops): Move to above location.

* subversion/libsvn_wc/wc.h (SVN_WC__ADM_ALL_WCPROPS): New macro.

* subversion/libsvn_wc/log.c
  (struct log_runner): Add wcprops_modified member.
  (log_do_modify_wcprop):  Set wcprops_modified instead of writing the new
  props to disk immediately.
  (run_log): Initialize wcprops_modified member of log_runner.  Write
  wcprops to disk if modified.

* subversion/libsvn_wc/adm_ops.c
  (svn_wc_remove_from_revision_control): Use svn_wc__remove_wcprops
  to remove wcprops.  When removing a directory, remove all wcprops in this
  directory (including file children) to avoid quadratic behaviour.

* subversion/libsvn_wc/adm_files.c
  (init_adm_tmp_area, init_adm): Don't create wcprops directory.

* subversion/libsvn_wc/lock.c
  (struct svn_wc_adm_access_t): Add wcprop member.
  (convert_wcprops): New function.
  (maybe_upgrade_format): Convert wcprop from old to new format.
  (adm_access_alloc): Init the wcprops member of the lock to NULL.
  (svn_wc__adm_access_set_wcprops): New function.
  (svn_wc__adm_access_wcprops): New function.

* subversion/libsvn_wc/lock.h
  (svn_wc__adm_access_set_wcprops): Declare.
  (svn_wc__adm_access_wcprops): Declare.
]]]

Re: [PATCH] Store wcprops in one file per .svn area

[Ivan Zhakov <ch...@gmail.com>]

On 4/15/06, Peter N. Lundblad <pe...@famlundblad.se> wrote:
> Hi,
>
> There is one more wc format change I'd like in 1.4.  (This is the last
> change, promise!).  The point is to avoid lots of wcprops files which
> are typically very small (100 bytes ore something) but still take up
> an inode and a disk block.
>
> To avoid O(n^2) complexity when setting wcprops during update/commit,
> I cache all wcprops in memory and write once at the end of the log run
> like we do with entries.  Switch currently has quadratic behaviour
> when invalidating all wcprops, but that's easily fixed.
>
> The patch is against the nonxml-entries branch because that was
> simplest for me, but it isn't dependent on that work.
>
> Any objections to the idea or to the patch in itself?
>
[...]
> Index: subversion/libsvn_wc/props.c
> ===================================================================
> --- subversion/libsvn_wc/props.c        (revision 19377)
> +++ subversion/libsvn_wc/props.c        (arbetskopia)
> @@ -2,7 +2,7 @@
>   * props.c :  routines dealing with properties in the working copy
>   *
>   * ====================================================================
> - * Copyright (c) 2000-2004 CollabNet.  All rights reserved.
> + * Copyright (c) 2000-2006 CollabNet.  All rights reserved.
>   *
>   * This software is licensed as described in the file COPYING, which
>   * you should have received as part of this distribution.  The terms
> @@ -826,36 +826,185 @@
>
>  /*** Private 'wc prop' functions ***/
>
> -/* A clone of svn_wc_prop_list, for the most part, except that it
> -   returns 'wc' props instead of normal props.  */
> +/* If wcprops are stored in a single file in this working copy, read that file
> +   and store it in the cache of ADM_ACCESS.   Use POOL for temporary
> +   allocations. */
>  static svn_error_t *
> -wcprop_list(apr_hash_t **props,
> -            const char *path,
> -            svn_wc_adm_access_t *adm_access,
> -            apr_pool_t *pool)
> +read_wcprops(svn_wc_adm_access_t *adm_access, apr_pool_t *pool)
>  {
> +  apr_file_t *file;
> +  apr_pool_t *cache_pool = svn_wc_adm_access_pool(adm_access);
> +  apr_hash_t *wcprops;
> +  /* Props for one entry. */
> +  apr_hash_t *proplist;
> +  svn_stream_t *stream;
> +  svn_error_t *err;
> +
> +  /* If the WC format is too old, there is nothing to cache. */
> +  /* ### XXX. Own constant? */
> +  if (svn_wc__adm_wc_format(adm_access) <= SVN_WC__XML_ENTRIES_VERSION)
> +    return SVN_NO_ERROR;
> +
> +  wcprops = apr_hash_make(cache_pool);
> +
> +  err = svn_wc__open_adm_file(&file, svn_wc_adm_access_path(adm_access),
> +                              SVN_WC__ADM_ALL_WCPROPS,
> +                              APR_READ | APR_BUFFERED, pool);
> +
> +  /* A non-existent file means there are no props. */
> +  if (err && err->apr_err == APR_ENOENT)
Should be APR_IS_STATUS_ENOENT

> +    {
> +      svn_error_clear(err);
> +      svn_wc__adm_access_set_wcprops(adm_access, wcprops);
> +      return SVN_NO_ERROR;
> +    }
> +  SVN_ERR(err);
> +
> +  stream = svn_stream_from_aprfile2(file, TRUE, pool);
> +
> +  /* Read the proplist for THIS_DIR. */
> +  proplist = apr_hash_make(cache_pool);
> +  SVN_ERR(svn_hash_read2(proplist, stream, SVN_HASH_TERMINATOR, cache_pool));
> +  apr_hash_set(wcprops, SVN_WC_ENTRY_THIS_DIR, APR_HASH_KEY_STRING, proplist);
[...]


With changing to APR_IS_STATUS_ENOENT, all tests passed on Windows
over ra_dav of course.
Peter, thanks for the good patch!

--
Ivan Zhakov