svn commit: r1648537 - in /subversion/trunk/subversion/libsvn_fs_fs: dag.c dag.h tree.c

[st...@apache.org]

Author: stefan2
Date: Tue Dec 30 15:05:41 2014
New Revision: 1648537

URL: http://svn.apache.org/r1648537
Log:
In FSFS, instead of locking the L1 DAG cache when handing out references
to its contents, copy the nodes into the result pool.  Because DAG nodes
are large objects (~1KB typ.), copy them only if they are not already in
the result pool.

This is the core patch for the memory growth issue caused by long-living
pools inadvertently holding locks on the cache.  The now unused locking
logic will be removed in later commits.

* subversion/libsvn_fs_fs/dag.h
  (svn_fs_fs__dag_copy_into_pool): New wrapper around svn_fs_fs__dag_dup
                                   that prevents unnecessary copies.

* subversion/libsvn_fs_fs/dag.c
  (svn_fs_fs__dag_copy_into_pool): Implement.

* subversion/libsvn_fs_fs/tree.c
  (dag_node_cache_get): Ignore the cache lock requests.  Cache locking
                        is no longer necessary.
  (make_parent_path,
   open_path): Ensure that all DAG nodes in the parent_path_t structure
               are allocated in the result pool.

Modified:
    subversion/trunk/subversion/libsvn_fs_fs/dag.c
    subversion/trunk/subversion/libsvn_fs_fs/dag.h
    subversion/trunk/subversion/libsvn_fs_fs/tree.c

Modified: subversion/trunk/subversion/libsvn_fs_fs/dag.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c?rev=1648537&r1=1648536&r2=1648537&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/dag.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/dag.c Tue Dec 30 15:05:41 2014
@@ -1105,6 +1105,15 @@ svn_fs_fs__dag_dup(const dag_node_t *nod
   return new_node;
 }
 
+dag_node_t *
+svn_fs_fs__dag_copy_into_pool(dag_node_t *node,
+                              apr_pool_t *pool)
+{
+  return (node->node_pool == pool
+            ? node
+            : svn_fs_fs__dag_dup(node, pool));
+}
+
 svn_error_t *
 svn_fs_fs__dag_serialize(void **data,
                          apr_size_t *data_len,

Modified: subversion/trunk/subversion/libsvn_fs_fs/dag.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.h?rev=1648537&r1=1648536&r2=1648537&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/dag.h (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/dag.h Tue Dec 30 15:05:41 2014
@@ -80,6 +80,12 @@ dag_node_t *
 svn_fs_fs__dag_dup(const dag_node_t *node,
                    apr_pool_t *pool);
 
+/* If NODE has been allocated in POOL, return NODE.  Otherwise, return
+   a copy created in POOL with svn_fs_fs__dag_dup. */
+dag_node_t *
+svn_fs_fs__dag_copy_into_pool(dag_node_t *node,
+                              apr_pool_t *pool);
+
 /* Serialize a DAG node, except don't try to preserve the 'fs' member.
    Implements svn_cache__serialize_func_t */
 svn_error_t *

Modified: subversion/trunk/subversion/libsvn_fs_fs/tree.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/tree.c?rev=1648537&r1=1648536&r2=1648537&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/tree.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/tree.c Tue Dec 30 15:05:41 2014
@@ -532,11 +532,6 @@ dag_node_cache_get(dag_node_t **node_p,
         {
           node = bucket->node;
         }
-
-      /* if we found a node, make sure it remains valid at least as long
-         as it would when allocated in POOL. */
-      if (node && needs_lock_cache)
-        lock_cache(ffd->dag_node_cache, pool);
     }
   else
     {
@@ -919,7 +914,8 @@ make_parent_path(dag_node_t *node,
                  apr_pool_t *pool)
 {
   parent_path_t *parent_path = apr_pcalloc(pool, sizeof(*parent_path));
-  parent_path->node = node;
+  if (node)
+    parent_path->node = svn_fs_fs__dag_copy_into_pool(node, pool);
   parent_path->entry = entry;
   parent_path->parent = parent;
   parent_path->copy_inherit = copy_id_inherit_unknown;
@@ -1197,7 +1193,7 @@ open_path(parent_path_t **parent_path_p,
           if (flags & open_path_node_only)
             {
               /* Shortcut: the caller only wants the final DAG node. */
-              parent_path->node = child;
+              parent_path->node = svn_fs_fs__dag_copy_into_pool(child, pool);
             }
           else
             {