You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2014/02/21 20:40:53 UTC

svn commit: r1570681 - in /subversion/trunk/subversion/libsvn_fs_x: cached_data.c caching.c fs.h fs_x.h transaction.c tree.c

Author: stefan2
Date: Fri Feb 21 19:40:53 2014
New Revision: 1570681

URL: http://svn.apache.org/r1570681
Log:
In FSX, remove the separate txn directory cache.

Because noderev IDs are now always (i.e. also between txns) globally
unique and different from all other item IDs such as representation IDs,
we can use it as a key with the existing directory cache. 

* subversion/libsvn_fs_x/fs.h
  (fs_x_data_t): Drop txn_dir_cache and the associated
                 concurrent_transactions member.

* subversion/libsvn_fs_x/fs_x.h
  (svn_fs_x__initialize_txn_caches,
   svn_fs_x__reset_txn_caches): No longer needed.

* subversion/libsvn_fs_x/caching.c
  (svn_fs_x__initialize_caches): We now use ID parts as key for the dir
                                 cache as they are the generalized form
                                 of the pair_cache_key_t.
  (struct txn_cleanup_baton_t,
   remove_txn_cache,
   init_txn_callbacks,
   svn_fs_x__initialize_txn_caches,
   svn_fs_x__reset_txn_caches): No longer needed.

* subversion/libsvn_fs_x/cached_data.c
  (locate_dir_cache): Return the key as struct. It is always an ID part now;
                      rep ID for committed dirs and noderev IDs while in txns.
  (svn_fs_x__rep_contents_dir,
   svn_fs_x__rep_contents_dir_entry): Update caller.

* subversion/libsvn_fs_x/transaction.c
  (purge_shared_txn_body): No need to handle txn_dir_cache anymore.
  (svn_fs_x__set_entry,
   svn_fs_x__delete_node_revision): Update the common dir cache now.

* subversion/libsvn_fs_x/tree.c
  (svn_fs_x__commit_txn,
   make_txn_root): Don't call txn_dir_cache functions anymore.

Modified:
    subversion/trunk/subversion/libsvn_fs_x/cached_data.c
    subversion/trunk/subversion/libsvn_fs_x/caching.c
    subversion/trunk/subversion/libsvn_fs_x/fs.h
    subversion/trunk/subversion/libsvn_fs_x/fs_x.h
    subversion/trunk/subversion/libsvn_fs_x/transaction.c
    subversion/trunk/subversion/libsvn_fs_x/tree.c

Modified: subversion/trunk/subversion/libsvn_fs_x/cached_data.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/cached_data.c?rev=1570681&r1=1570680&r2=1570681&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/cached_data.c (original)
+++ subversion/trunk/subversion/libsvn_fs_x/cached_data.c Fri Feb 21 19:40:53 2014
@@ -2033,43 +2033,38 @@ parse_dir_entries(apr_hash_t **entries_p
 }
 
 /* Return the cache object in FS responsible to storing the directory the
- * NODEREV plus the corresponding *KEY.  If no cache exists, return NULL.
- * PAIR_KEY must point to some key struct, which does not need to be
- * initialized.  We use it to avoid dynamic allocation.
+ * NODEREV plus the corresponding pre-allocated *KEY.
  */
 static svn_cache__t *
 locate_dir_cache(svn_fs_t *fs,
-                 const void **key,
-                 pair_cache_key_t *pair_key,
+                 svn_fs_x__id_part_t *key,
                  node_revision_t *noderev,
                  apr_pool_t *pool)
 {
   fs_x_data_t *ffd = fs->fsap_data;
   if (svn_fs_x__id_is_txn(noderev->id))
     {
-      /* data in txns requires the expensive fs_id-based addressing mode */
-      *key = svn_fs_x__id_unparse(noderev->id, pool)->data;
-      return ffd->txn_dir_cache;
+      /* data in txns must be addressed by ID since the representation has
+         not been created, yet. */
+      *key = *svn_fs_x__id_noderev_id(noderev->id);
     }
   else
     {
       /* committed data can use simple rev,item pairs */
       if (noderev->data_rep)
         {
-          pair_key->revision
-            = svn_fs_x__get_revnum(noderev->data_rep->id.change_set);
-          pair_key->second = noderev->data_rep->id.number;
-          *key = pair_key;
+          *key = noderev->data_rep->id;
         }
       else
         {
           /* no data rep -> empty directory.
-             A NULL key causes a cache miss. */
-          *key = NULL;
+             Use a key that does definitely not clash with non-NULL reps. */
+          key->change_set = SVN_FS_X__INVALID_CHANGE_SET;
+          key->number = SVN_FS_X__ITEM_INDEX_UNUSED;
         }
-
-      return ffd->dir_cache;
     }
+
+  return ffd->dir_cache;
 }
 
 svn_error_t *
@@ -2078,17 +2073,16 @@ svn_fs_x__rep_contents_dir(apr_hash_t **
                            node_revision_t *noderev,
                            apr_pool_t *pool)
 {
-  pair_cache_key_t pair_key = { 0 };
-  const void *key;
+  svn_fs_x__id_part_t key;
   apr_hash_t *unparsed_entries, *parsed_entries;
 
   /* find the cache we may use */
-  svn_cache__t *cache = locate_dir_cache(fs, &key, &pair_key, noderev, pool);
+  svn_cache__t *cache = locate_dir_cache(fs, &key, noderev, pool);
   if (cache)
     {
       svn_boolean_t found;
 
-      SVN_ERR(svn_cache__get((void **)entries_p, &found, cache, key, pool));
+      SVN_ERR(svn_cache__get((void **)entries_p, &found, cache, &key, pool));
       if (found)
         return SVN_NO_ERROR;
     }
@@ -2101,7 +2095,7 @@ svn_fs_x__rep_contents_dir(apr_hash_t **
 
   /* Update the cache, if we are to use one. */
   if (cache)
-    SVN_ERR(svn_cache__set(cache, key, parsed_entries, pool));
+    SVN_ERR(svn_cache__set(cache, &key, parsed_entries, pool));
 
   *entries_p = parsed_entries;
   return SVN_NO_ERROR;
@@ -2118,17 +2112,15 @@ svn_fs_x__rep_contents_dir_entry(svn_fs_
   svn_boolean_t found = FALSE;
 
   /* find the cache we may use */
-  pair_cache_key_t pair_key = { 0 };
-  const void *key;
-  svn_cache__t *cache = locate_dir_cache(fs, &key, &pair_key, noderev,
-                                         scratch_pool);
+  svn_fs_x__id_part_t key;
+  svn_cache__t *cache = locate_dir_cache(fs, &key, noderev, scratch_pool);
   if (cache)
     {
       /* Cache lookup. */
       SVN_ERR(svn_cache__get_partial((void **)dirent,
                                      &found,
                                      cache,
-                                     key,
+                                     &key,
                                      svn_fs_x__extract_dir_entry,
                                      (void*)name,
                                      result_pool));
@@ -2144,7 +2136,7 @@ svn_fs_x__rep_contents_dir_entry(svn_fs_
       /* read the dir from the file system. It will probably be put it
          into the cache for faster lookup in future calls. */
       SVN_ERR(svn_fs_x__rep_contents_dir(&entries, fs, noderev,
-                                          scratch_pool));
+                                         scratch_pool));
 
       /* find desired entry and return a copy in POOL, if found */
       entry = svn_hash_gets(entries, name);

Modified: subversion/trunk/subversion/libsvn_fs_x/caching.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/caching.c?rev=1570681&r1=1570680&r2=1570681&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/caching.c (original)
+++ subversion/trunk/subversion/libsvn_fs_x/caching.c Fri Feb 21 19:40:53 2014
@@ -469,7 +469,7 @@ svn_fs_x__initialize_caches(svn_fs_t *fs
                        1024, 8,
                        svn_fs_x__serialize_dir_entries,
                        svn_fs_x__deserialize_dir_entries,
-                       sizeof(pair_cache_key_t),
+                       sizeof(svn_fs_x__id_part_t),
                        apr_pstrcat(pool, prefix, "DIR", SVN_VA_NULL),
                        SVN_CACHE__MEMBUFFER_LOW_PRIORITY,
                        fs,
@@ -754,114 +754,3 @@ svn_fs_x__initialize_caches(svn_fs_t *fs
 
   return SVN_NO_ERROR;
 }
-
-/* Baton to be used for the remove_txn_cache() pool cleanup function, */
-struct txn_cleanup_baton_t
-{
-  /* the cache to reset */
-  svn_cache__t *txn_cache;
-
-  /* the position where to reset it */
-  svn_cache__t **to_reset;
-};
-
-/* APR pool cleanup handler that will reset the cache pointer given in
-   BATON_VOID. */
-static apr_status_t
-remove_txn_cache(void *baton_void)
-{
-  struct txn_cleanup_baton_t *baton = baton_void;
-
-  /* be careful not to hurt performance by resetting newer txn's caches. */
-  if (*baton->to_reset == baton->txn_cache)
-    {
-     /* This is equivalent to calling svn_fs_x__reset_txn_caches(). */
-      *baton->to_reset  = NULL;
-    }
-
-  return  APR_SUCCESS;
-}
-
-/* This function sets / registers the required callbacks for a given
- * transaction-specific *CACHE object, if CACHE is not NULL and a no-op
- * otherwise. In particular, it will ensure that *CACHE gets reset to NULL
- * upon POOL destruction latest.
- */
-static void
-init_txn_callbacks(svn_cache__t **cache,
-                   apr_pool_t *pool)
-{
-  if (*cache != NULL)
-    {
-      struct txn_cleanup_baton_t *baton;
-
-      baton = apr_palloc(pool, sizeof(*baton));
-      baton->txn_cache = *cache;
-      baton->to_reset = cache;
-
-      apr_pool_cleanup_register(pool,
-                                baton,
-                                remove_txn_cache,
-                                apr_pool_cleanup_null);
-    }
-}
-
-svn_error_t *
-svn_fs_x__initialize_txn_caches(svn_fs_t *fs,
-                                const char *txn_id,
-                                apr_pool_t *pool)
-{
-  fs_x_data_t *ffd = fs->fsap_data;
-
-  /* Transaction content needs to be carefully prefixed to virtually
-     eliminate any chance for conflicts. The (repo, txn_id) pair
-     should be unique but if a transaction fails, it might be possible
-     to start a new transaction later that receives the same id.
-     Therefore, throw in a uuid as well - just to be sure. */
-  const char *prefix = apr_pstrcat(pool,
-                                   "fsx:", fs->uuid,
-                                   "/", fs->path,
-                                   ":", txn_id,
-                                   ":", svn_uuid_generate(pool), ":",
-                                   SVN_VA_NULL);
-
-  /* We don't support caching for concurrent transactions in the SAME
-   * FSX session. Maybe, you forgot to clean POOL. */
-  if (ffd->txn_dir_cache != NULL || ffd->concurrent_transactions)
-    {
-      ffd->txn_dir_cache = NULL;
-      ffd->concurrent_transactions = TRUE;
-
-      return SVN_NO_ERROR;
-    }
-
-  /* create a txn-local directory cache */
-  SVN_ERR(create_cache(&ffd->txn_dir_cache,
-                       NULL,
-                       svn_cache__get_global_membuffer_cache(),
-                       1024, 8,
-                       svn_fs_x__serialize_dir_entries,
-                       svn_fs_x__deserialize_dir_entries,
-                       APR_HASH_KEY_STRING,
-                       apr_pstrcat(pool, prefix, "TXNDIR",
-                                   SVN_VA_NULL),
-                       0,
-                       fs,
-                       TRUE,
-                       pool));
-
-  /* reset the transaction-specific cache if the pool gets cleaned up. */
-  init_txn_callbacks(&(ffd->txn_dir_cache), pool);
-
-  return SVN_NO_ERROR;
-}
-
-void
-svn_fs_x__reset_txn_caches(svn_fs_t *fs)
-{
-  /* we can always just reset the caches. This may degrade performance but
-   * can never cause in incorrect behavior. */
-
-  fs_x_data_t *ffd = fs->fsap_data;
-  ffd->txn_dir_cache = NULL;
-}

Modified: subversion/trunk/subversion/libsvn_fs_x/fs.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/fs.h?rev=1570681&r1=1570680&r2=1570681&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/fs.h (original)
+++ subversion/trunk/subversion/libsvn_fs_x/fs.h Fri Feb 21 19:40:53 2014
@@ -363,13 +363,6 @@ typedef struct fs_x_data_t
   /* TRUE while the we hold a lock on the write lock file. */
   svn_boolean_t has_write_lock;
 
-  /* If set, there are or have been more than one concurrent transaction */
-  svn_boolean_t concurrent_transactions;
-
-  /* Temporary cache for changed directories yet to be committed; maps from
-     unparsed FS ID to ###x.  NULL outside transactions. */
-  svn_cache__t *txn_dir_cache;
-
   /* Data shared between all svn_fs_t objects for a given filesystem. */
   fs_x_shared_data_t *shared;
 

Modified: subversion/trunk/subversion/libsvn_fs_x/fs_x.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/fs_x.h?rev=1570681&r1=1570680&r2=1570681&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/fs_x.h (original)
+++ subversion/trunk/subversion/libsvn_fs_x/fs_x.h Fri Feb 21 19:40:53 2014
@@ -214,20 +214,4 @@ svn_fs_x__get_node_origin(const svn_fs_i
 svn_error_t *
 svn_fs_x__initialize_caches(svn_fs_t *fs, apr_pool_t *pool);
 
-/* Initialize all transaction-local caches in FS according to the global
-   cache settings and make TXN_ID part of their key space. Use POOL for
-   allocations.
-
-   Please note that it is permissible for this function to set some or all
-   of these caches to NULL, regardless of any setting. */
-svn_error_t *
-svn_fs_x__initialize_txn_caches(svn_fs_t *fs,
-                                const char *txn_id,
-                                apr_pool_t *pool);
-
-/* Resets the svn_cache__t structures local to the current transaction in FS.
-   Calling it more than once per txn or from outside any txn is allowed. */
-void
-svn_fs_x__reset_txn_caches(svn_fs_t *fs);
-
 #endif

Modified: subversion/trunk/subversion/libsvn_fs_x/transaction.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/transaction.c?rev=1570681&r1=1570680&r2=1570681&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/transaction.c (original)
+++ subversion/trunk/subversion/libsvn_fs_x/transaction.c Fri Feb 21 19:40:53 2014
@@ -513,7 +513,6 @@ purge_shared_txn_body(svn_fs_t *fs, cons
   svn_fs_x__txn_id_t txn_id = *(const svn_fs_x__txn_id_t *)baton;
 
   free_shared_txn(fs, txn_id);
-  svn_fs_x__reset_txn_caches(fs);
 
   return SVN_NO_ERROR;
 }
@@ -1541,12 +1540,11 @@ svn_fs_x__set_entry(svn_fs_t *fs,
       out = svn_stream_from_aprfile2(file, TRUE, pool);
     }
 
-  /* if we have a directory cache for this transaction, update it */
-  if (ffd->txn_dir_cache)
+  /* update directory cache */
     {
       /* build parameters: (name, new entry) pair */
-      const char *key =
-          svn_fs_x__id_unparse(parent_noderev->id, subpool)->data;
+      const svn_fs_x__id_part_t *key
+        = svn_fs_x__id_noderev_id(parent_noderev->id);
       replace_baton_t baton;
 
       baton.name = name;
@@ -1561,7 +1559,7 @@ svn_fs_x__set_entry(svn_fs_t *fs,
         }
 
       /* actually update the cached directory (if cached) */
-      SVN_ERR(svn_cache__set_partial(ffd->txn_dir_cache, key,
+      SVN_ERR(svn_cache__set_partial(ffd->dir_cache, key,
                                      svn_fs_x__replace_dir_entry, &baton,
                                      subpool));
     }
@@ -3430,16 +3428,14 @@ svn_fs_x__delete_node_revision(svn_fs_t 
       && noderev->kind == svn_node_dir)
     {
       fs_x_data_t *ffd = fs->fsap_data;
+      const svn_fs_x__id_part_t *key = svn_fs_x__id_noderev_id(id);
+
       SVN_ERR(svn_io_remove_file2(svn_fs_x__path_txn_node_children(fs, id,
                                                                    pool),
                                   FALSE, pool));
 
       /* remove the corresponding entry from the cache, if such exists */
-      if (ffd->txn_dir_cache)
-        {
-          const char *key = svn_fs_x__id_unparse(id, pool)->data;
-          SVN_ERR(svn_cache__set(ffd->txn_dir_cache, key, NULL, pool));
-        }
+      SVN_ERR(svn_cache__set(ffd->dir_cache, key, NULL, pool));
     }
 
   return svn_io_remove_file2(svn_fs_x__path_txn_node_rev(fs, id, pool),

Modified: subversion/trunk/subversion/libsvn_fs_x/tree.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/tree.c?rev=1570681&r1=1570680&r2=1570681&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/tree.c (original)
+++ subversion/trunk/subversion/libsvn_fs_x/tree.c Fri Feb 21 19:40:53 2014
@@ -2227,8 +2227,6 @@ svn_fs_x__commit_txn(const char **confli
 
  cleanup:
 
-  svn_fs_x__reset_txn_caches(fs);
-
   svn_pool_destroy(iterpool);
   return svn_error_trace(err);
 }
@@ -4273,12 +4271,6 @@ make_txn_root(svn_fs_root_t **root_p,
                                                   SVN_VA_NULL),
                                       root->pool));
 
-  /* Initialize transaction-local caches in FS.
-
-     Note that we cannot put those caches in frd because that content
-     fs root object is not available where we would need it. */
-  SVN_ERR(svn_fs_x__initialize_txn_caches(fs, root->txn, root->pool));
-
   root->fsap_data = frd;
 
   *root_p = root;