subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From stef...@apache.org
Subject svn commit: r1745852 - in /subversion/trunk/subversion/libsvn_fs_fs: cached_data.c cached_data.h fs.h stats.c transaction.c tree.c
Date Sat, 28 May 2016 08:45:57 GMT
Author: stefan2
Date: Sat May 28 08:45:57 2016
New Revision: 1745852

URL: http://svn.apache.org/viewvc?rev=1745852&view=rev
Log:
In FSFS, introduce the concept of a get_changes context.
Everything above svn_fs_fs__get_changes is now "final".

This replicates most of r1733804 and its follow-up r1733830.

* subversion/libsvn_fs_fs/fs.h
  (svn_fs_fs__changes_context_t): Define new internal data type.

* subversion/libsvn_fs_fs/cached_data.h
  (svn_fs_fs__create_changes_context): Declare new constructor.
  (svn_fs_fs__get_changes): Take inputs from the CONTEXT now and use the
                            two-pool paradigm.

* subversion/libsvn_fs_fs/cached_data.c
  (svn_fs_fs__create_changes_context): Implement.
  (svn_fs_fs__get_changes): CONTEXT provides most of the parameters and
                            variables now.  SCRATCH_POOL is provided by
                            the caller.  Be sure to update the CONTEXT
                            according to the *CHANGES returned and make
                            sure to close the rev file in the last call.

* subversion/libsvn_fs_fs/stats.c
  (get_phys_change_count): Update caller.  We can now use a normal ITERPOOL
                           Instead of a SUBPOOL.

* subversion/libsvn_fs_fs/transaction.c
  (svn_fs_fs__paths_changed): Update the backward compat code.

* subversion/libsvn_fs_fs/tree.c
  (fs_revision_changes_iterator_data_t,
   fs_revision_changes_iterator_get,
   fs_report_changes): Fetch the changes iteratively now - even though the
                       underlying layer will return them in one block, ATM.

Modified:
    subversion/trunk/subversion/libsvn_fs_fs/cached_data.c
    subversion/trunk/subversion/libsvn_fs_fs/cached_data.h
    subversion/trunk/subversion/libsvn_fs_fs/fs.h
    subversion/trunk/subversion/libsvn_fs_fs/stats.c
    subversion/trunk/subversion/libsvn_fs_fs/transaction.c
    subversion/trunk/subversion/libsvn_fs_fs/tree.c

Modified: subversion/trunk/subversion/libsvn_fs_fs/cached_data.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/cached_data.c?rev=1745852&r1=1745851&r2=1745852&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/cached_data.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/cached_data.c Sat May 28 08:45:57 2016
@@ -2884,23 +2884,37 @@ svn_fs_fs__get_proplist(apr_hash_t **pro
 }
 
 svn_error_t *
+svn_fs_fs__create_changes_context(svn_fs_fs__changes_context_t **context,
+                                  svn_fs_t *fs,
+                                  svn_revnum_t rev,
+                                  apr_pool_t *result_pool)
+{
+  svn_fs_fs__changes_context_t *result = apr_pcalloc(result_pool,
+                                                     sizeof(*result));
+  result->fs = fs;
+  result->revision = rev;
+  result->rev_file_pool = result_pool;
+
+  *context = result;
+  return SVN_NO_ERROR;
+}
+
+svn_error_t *
 svn_fs_fs__get_changes(apr_array_header_t **changes,
-                       svn_fs_t *fs,
-                       svn_revnum_t rev,
-                       apr_pool_t *result_pool)
+                       svn_fs_fs__changes_context_t *context,
+                       apr_pool_t *result_pool,
+                       apr_pool_t *scratch_pool)
 {
   apr_off_t item_index = SVN_FS_FS__ITEM_INDEX_CHANGES;
-  svn_fs_fs__revision_file_t *revision_file;
   svn_boolean_t found;
-  fs_fs_data_t *ffd = fs->fsap_data;
-  apr_pool_t *scratch_pool = svn_pool_create(result_pool);
+  fs_fs_data_t *ffd = context->fs->fsap_data;
 
   /* try cache lookup first */
 
   if (ffd->changes_cache)
     {
       SVN_ERR(svn_cache__get((void **) changes, &found, ffd->changes_cache,
-                             &rev, result_pool));
+                             &context->revision, result_pool));
     }
   else
     {
@@ -2911,16 +2925,25 @@ svn_fs_fs__get_changes(apr_array_header_
     {
       /* read changes from revision file */
 
-      SVN_ERR(svn_fs_fs__ensure_revision_exists(rev, fs, scratch_pool));
-      SVN_ERR(svn_fs_fs__open_pack_or_rev_file(&revision_file, fs, rev,
-                                               scratch_pool, scratch_pool));
+      if (!context->revision_file)
+        {
+          SVN_ERR(svn_fs_fs__ensure_revision_exists(context->revision,
+                                                    context->fs,
+                                                    scratch_pool));
+          SVN_ERR(svn_fs_fs__open_pack_or_rev_file(&context->revision_file,
+                                                   context->fs,
+                                                   context->revision,
+                                                   context->rev_file_pool,
+                                                   scratch_pool));
+        }
 
-      if (use_block_read(fs))
+      if (use_block_read(context->fs))
         {
           /* 'block-read' will also provide us with the desired data */
-          SVN_ERR(block_read((void **)changes, fs,
-                             rev, SVN_FS_FS__ITEM_INDEX_CHANGES,
-                             revision_file, result_pool, scratch_pool));
+          SVN_ERR(block_read((void **)changes, context->fs,
+                             context->revision, SVN_FS_FS__ITEM_INDEX_CHANGES,
+                             context->revision_file, result_pool,
+                             scratch_pool));
         }
       else
         {
@@ -2928,17 +2951,19 @@ svn_fs_fs__get_changes(apr_array_header_
 
           /* Addressing is very different for old formats
            * (needs to read the revision trailer). */
-          if (svn_fs_fs__use_log_addressing(fs))
+          if (svn_fs_fs__use_log_addressing(context->fs))
             {
-              SVN_ERR(svn_fs_fs__item_offset(&changes_offset, fs,
-                                             revision_file, rev, NULL,
+              SVN_ERR(svn_fs_fs__item_offset(&changes_offset, context->fs,
+                                             context->revision_file,
+                                             context->revision, NULL,
                                              SVN_FS_FS__ITEM_INDEX_CHANGES,
                                              scratch_pool));
             }
           else
             {
               SVN_ERR(get_root_changes_offset(NULL, &changes_offset,
-                                              revision_file, fs, rev,
+                                              context->revision_file,
+                                              context->fs, context->revision,
                                               scratch_pool));
 
               /* This variable will be used for debug logging only. */
@@ -2946,9 +2971,10 @@ svn_fs_fs__get_changes(apr_array_header_
             }
 
           /* Actual reading and parsing are the same, though. */
-          SVN_ERR(aligned_seek(fs, revision_file->file, NULL, changes_offset,
-                               scratch_pool));
-          SVN_ERR(svn_fs_fs__read_changes(changes, revision_file->stream,
+          SVN_ERR(aligned_seek(context->fs, context->revision_file->file,
+                               NULL, changes_offset, scratch_pool));
+          SVN_ERR(svn_fs_fs__read_changes(changes,
+                                          context->revision_file->stream,
                                           result_pool, scratch_pool));
 
           /* cache for future reference */
@@ -2963,18 +2989,26 @@ svn_fs_fs__get_changes(apr_array_header_
                * large, memory is scarce or both.  Having a huge temporary
                * copy would not be a good thing in either case. */
               if (svn_cache__is_cachable(ffd->changes_cache, estimated_size))
-                SVN_ERR(svn_cache__set(ffd->changes_cache, &rev, *changes,
-                                       scratch_pool));
+                SVN_ERR(svn_cache__set(ffd->changes_cache, &context->revision,
+                                       *changes, scratch_pool));
             }
         }
+    }
 
-      SVN_ERR(svn_fs_fs__close_revision_file(revision_file));
+  /* TODO: This is transitional code ... */
+  context->next += (*changes)->nelts;
+  context->eol = TRUE;
+
+  /* Close the revision file after we read all data. */
+  if (context->eol && context->revision_file)
+    {
+      SVN_ERR(svn_fs_fs__close_revision_file(context->revision_file));
+      context->revision_file = NULL;
     }
 
-  SVN_ERR(dbg_log_access(fs, rev, item_index, *changes,
+  SVN_ERR(dbg_log_access(context->fs, context->revision, item_index, *changes,
                          SVN_FS_FS__ITEM_TYPE_CHANGES, scratch_pool));
 
-  svn_pool_destroy(scratch_pool);
   return SVN_NO_ERROR;
 }
 

Modified: subversion/trunk/subversion/libsvn_fs_fs/cached_data.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/cached_data.h?rev=1745852&r1=1745851&r2=1745852&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/cached_data.h (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/cached_data.h Sat May 28 08:45:57 2016
@@ -158,13 +158,22 @@ svn_fs_fs__get_proplist(apr_hash_t **pro
                         node_revision_t *noderev,
                         apr_pool_t *pool);
 
-/* Fetch the list of change in revision REV in FS and return it in *CHANGES.
- * Allocate the result in POOL.
+/* Create a changes retrieval context object in *RESULT_POOL and return it
+ * in *CONTEXT.  It will allow svn_fs_x__get_changes to fetch consecutive
+ * blocks (one per invocation) from REV's changed paths list in FS. */
+svn_error_t *
+svn_fs_fs__create_changes_context(svn_fs_fs__changes_context_t **context,
+                                  svn_fs_t *fs,
+                                  svn_revnum_t rev,
+                                  apr_pool_t *result_pool);
+
+/* Fetch the block of changes from the CONTEXT and return it in *CHANGES.
+ * Allocate the result in RESULT_POOL and use SCRATCH_POOL for temporaries.
  */
 svn_error_t *
 svn_fs_fs__get_changes(apr_array_header_t **changes,
-                       svn_fs_t *fs,
-                       svn_revnum_t rev,
-                       apr_pool_t *pool);
+                       svn_fs_fs__changes_context_t *context,
+                       apr_pool_t *result_pool,
+                       apr_pool_t *scratch_pool);
 
 #endif

Modified: subversion/trunk/subversion/libsvn_fs_fs/fs.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs.h?rev=1745852&r1=1745851&r2=1745852&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/fs.h (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/fs.h Sat May 28 08:45:57 2016
@@ -37,7 +37,7 @@
 #include "private/svn_sqlite.h"
 #include "private/svn_mutex.h"
 
-#include "id.h"
+#include "rev_file.h"
 
 #ifdef __cplusplus
 extern "C" {
@@ -624,6 +624,29 @@ typedef struct change_t
 } change_t;
 
 
+/*** Context for reading changed paths lists iteratively. */
+typedef struct svn_fs_fs__changes_context_t
+{
+  /* Repository to fetch from. */
+  svn_fs_t *fs;
+
+  /* Revision that we read from. */
+  svn_revnum_t revision;
+
+  /* Revision file object to use when needed.  NULL until the first access. */
+  svn_fs_fs__revision_file_t *revision_file;
+
+  /* Pool to create REVISION_FILE in. */
+  apr_pool_t *rev_file_pool;
+
+  /* Index of the next change to fetch. */
+  apr_size_t next;
+
+  /* Has the end of the list been reached? */
+  svn_boolean_t eol;
+
+} svn_fs_fs__changes_context_t;
+
 /*** Directory (only used at the cache interface) ***/
 typedef struct svn_fs_fs__dir_data_t
 {

Modified: subversion/trunk/subversion/libsvn_fs_fs/stats.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/stats.c?rev=1745852&r1=1745851&r2=1745852&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/stats.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/stats.c Sat May 28 08:45:57 2016
@@ -681,19 +681,25 @@ get_phys_change_count(query_t *query,
                       revision_info_t *revision_info,
                       apr_pool_t *scratch_pool)
 {
-  /* We are going to use our own sub-pool here because the changes object
-   * may well be >100MB and SCRATCH_POOL may not get cleared until all other
-   * info has been read by read_phys_revision().  Therefore, tidy up early.
-   */
-  apr_pool_t *subpool = svn_pool_create(scratch_pool);
-  apr_array_header_t *changes;
+  apr_pool_t *iterpool = svn_pool_create(scratch_pool);
+  svn_fs_fs__changes_context_t *context;
 
-  SVN_ERR(svn_fs_fs__get_changes(&changes, query->fs,
-                                 revision_info->revision, subpool));
-  revision_info->change_count = changes->nelts;
+  /* Fetch the first block of data. */
+  SVN_ERR(svn_fs_fs__create_changes_context(&context, query->fs,
+                                            revision_info->revision,
+                                            scratch_pool));
 
-  /* Release potentially tons of memory. */
-  svn_pool_destroy(subpool);
+  revision_info->change_count = 0;
+  while (!context->eol)
+    {
+      apr_array_header_t *changes;
+
+      svn_pool_clear(iterpool);
+      SVN_ERR(svn_fs_fs__get_changes(&changes, context, iterpool, iterpool));
+      revision_info->change_count = changes->nelts;
+    }
+
+  svn_pool_destroy(iterpool);
 
   return SVN_NO_ERROR;
 }

Modified: subversion/trunk/subversion/libsvn_fs_fs/transaction.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/transaction.c?rev=1745852&r1=1745851&r2=1745852&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/transaction.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/transaction.c Sat May 28 08:45:57 2016
@@ -932,20 +932,36 @@ svn_fs_fs__paths_changed(apr_hash_t **ch
                          svn_revnum_t rev,
                          apr_pool_t *pool)
 {
-  apr_hash_t *changed_paths;
-  apr_array_header_t *changes;
-  int i;
+  apr_hash_t *changed_paths = svn_hash__make(pool);
+  svn_fs_fs__changes_context_t *context;
 
-  SVN_ERR(svn_fs_fs__get_changes(&changes, fs, rev, pool));
+  apr_pool_t *iterpool = svn_pool_create(pool);
 
-  changed_paths = svn_hash__make(pool);
-  for (i = 0; i < changes->nelts; ++i)
+  /* Fetch all data block-by-block. */
+  SVN_ERR(svn_fs_fs__create_changes_context(&context, fs, rev, pool));
+  while (!context->eol)
     {
-      change_t *change = APR_ARRAY_IDX(changes, i, change_t *);
-      apr_hash_set(changed_paths, change->path.data, change->path.len,
-                   &change->info);
+      apr_array_header_t *changes;
+      int i;
+
+      svn_pool_clear(iterpool);
+
+      /* Be sure to allocate the changes in the result POOL, even though
+         we don't need the array itself afterwards.  Copying the entries
+         from a temp pool to the result POOL would be expensive and saves
+         use less then 10% memory. */
+      SVN_ERR(svn_fs_fs__get_changes(&changes, context, pool, iterpool));
+
+      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);
+        }
     }
 
+  svn_pool_destroy(iterpool);
+
   *changed_paths_p = changed_paths;
 
   return SVN_NO_ERROR;

Modified: subversion/trunk/subversion/libsvn_fs_fs/tree.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/tree.c?rev=1745852&r1=1745851&r2=1745852&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/tree.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/tree.c Sat May 28 08:45:57 2016
@@ -3346,6 +3346,10 @@ static changes_iterator_vtable_t txn_cha
 /* FSAP data structure for in-revision changes list iterators. */
 typedef struct fs_revision_changes_iterator_data_t
 {
+  /* Context that tells the lower layers from where to fetch the next
+     block of changes. */
+  svn_fs_fs__changes_context_t *context;
+
   /* Changes to send. */
   apr_array_header_t *changes;
 
@@ -3355,6 +3359,10 @@ typedef struct fs_revision_changes_itera
   /* For efficiency such that we don't need to dynamically allocate
      yet another copy of that data. */
   svn_fs_path_change3_t change;
+
+  /* A cleanable scratch pool in case we need one.
+     No further sub-pool creation necessary. */
+  apr_pool_t *scratch_pool;
 } fs_revision_changes_iterator_data_t;
 
 /* Implement changes_iterator_vtable_t.get for in-revision change lists. */
@@ -3364,6 +3372,22 @@ fs_revision_changes_iterator_get(svn_fs_
 {
   fs_revision_changes_iterator_data_t *data = iterator->fsap_data;
 
+  /* If we exhausted our block of changes and did not reach the end of the
+     list, yet, fetch the next block.  Note that that block may be empty. */
+  if ((data->idx >= data->changes->nelts) && !data->context->eol)
+    {
+      apr_pool_t *changes_pool = data->changes->pool;
+
+      /* Drop old changes block, read new block. */
+      svn_pool_clear(changes_pool);
+      SVN_ERR(svn_fs_fs__get_changes(&data->changes, data->context,
+                                     changes_pool, data->scratch_pool));
+      data->idx = 0;
+
+      /* Immediately release any temporary data. */
+      svn_pool_clear(data->scratch_pool);
+    }
+
   if (data->idx < data->changes->nelts)
     {
       change_t *entry = APR_ARRAY_IDX(data->changes, data->idx, change_t *);
@@ -3408,11 +3432,27 @@ fs_report_changes(svn_fs_path_change_ite
     }
   else
     {
+      /* The block of changes that we retrieve need to live in a separately
+         cleanable pool. */
+      apr_pool_t *changes_pool = svn_pool_create(result_pool);
+
+      /* Our iteration context info. */
       fs_revision_changes_iterator_data_t *data = apr_pcalloc(result_pool,
                                                               sizeof(*data));
-      SVN_ERR(svn_fs_fs__get_changes(&data->changes, root->fs, root->rev,
-                                     result_pool));
 
+      /* This pool must remain valid as long as ITERATOR lives but will
+         be used only for temporary allocations and will be cleaned up
+         frequently.  So, this must be a sub-pool of RESULT_POOL. */
+      data->scratch_pool = svn_pool_create(result_pool);
+
+      /* Fetch the first block of data. */
+      SVN_ERR(svn_fs_fs__create_changes_context(&data->context,
+                                                root->fs, root->rev,
+                                                result_pool));
+      SVN_ERR(svn_fs_fs__get_changes(&data->changes, data->context,
+                                     changes_pool, scratch_pool));
+
+      /* Return the fully initialized object. */
       result->fsap_data = data;
       result->vtable = &rev_changes_iterator_vtable;
     }



Mime
View raw message