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 2016/03/06 13:21:51 UTC

svn commit: r1733804 - in /subversion/trunk/subversion/libsvn_fs_x: cached_data.c cached_data.h fs.h tree.c

Author: stefan2
Date: Sun Mar  6 12:21:50 2016
New Revision: 1733804

URL: http://svn.apache.org/viewvc?rev=1733804&view=rev
Log:
In FSX, make the internal changed paths retrieval API streamy.

Instead of returning full lists, the new cached data layer API will
return only 1 block of changes, e.g. 100, and deliver more upon the
next call.  The size of a block is unspecified at the API level.
The current position within the changes list plus a number of other
context data is stored in a newly introduced context object.

This does not change the actual retrieval but above the cached data
layer, the code is now "final".  Also, the code path for in-txn data
will remain as-is because it needs to sanitize the whole changes list
before returning it and that can't be done iteratively.

* subversion/libsvn_fs_x/fs.h
  (svn_fs_x__changes_context_t): Define new internal API struct.

* subversion/libsvn_fs_x/cached_data.h
  (svn_fs_x__create_changes_context): Declare new internal API.
  (svn_fs_x__get_changes): Use two-pool paradigm now and take a
                           CONTEXT instead of FS+REV.

* subversion/libsvn_fs_x/cached_data.c
  (svn_fs_x__get_proplist): Implement.
  (svn_fs_x__get_changes): Take info from the CONTEXT now and update
                           the CONTEXT upon exit.

* subversion/libsvn_fs_x/tree.c
  (fs_revision_changes_iterator_data_t): Add the new retrieval context
                                         plus a temp pool.
  (x_revision_changes_iterator_get): Request data block by block.
  (x_report_changes): Update, performing the more complex query setup.

Modified:
    subversion/trunk/subversion/libsvn_fs_x/cached_data.c
    subversion/trunk/subversion/libsvn_fs_x/cached_data.h
    subversion/trunk/subversion/libsvn_fs_x/fs.h
    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=1733804&r1=1733803&r2=1733804&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/cached_data.c (original)
+++ subversion/trunk/subversion/libsvn_fs_x/cached_data.c Sun Mar  6 12:21:50 2016
@@ -2801,39 +2801,52 @@ svn_fs_x__get_proplist(apr_hash_t **prop
   return SVN_NO_ERROR;
 }
 
+svn_error_t *
+svn_fs_x__create_changes_context(svn_fs_x__changes_context_t **context,
+                                 svn_fs_t *fs,
+                                 svn_revnum_t rev,
+                                 apr_pool_t *result_pool,
+                                 apr_pool_t *scratch_pool)
+{
+  svn_fs_x__changes_context_t *result = apr_pcalloc(result_pool,
+                                                    sizeof(*result));
+  result->fs = fs;
+  result->revision = rev;
+
+  SVN_ERR(svn_fs_x__ensure_revision_exists(rev, fs, scratch_pool));
+  SVN_ERR(svn_fs_x__rev_file_init(&result->revision_file, fs, rev,
+                                  result_pool));
 
+  *context = result;
+  return SVN_NO_ERROR;
+}
 
 svn_error_t *
 svn_fs_x__get_changes(apr_array_header_t **changes,
-                      svn_fs_t *fs,
-                      svn_revnum_t rev,
-                      apr_pool_t *result_pool)
+                      svn_fs_x__changes_context_t *context,
+                      apr_pool_t *result_pool,
+                      apr_pool_t *scratch_pool)
 {
-  svn_fs_x__revision_file_t *revision_file;
   svn_boolean_t found;
-  svn_fs_x__data_t *ffd = fs->fsap_data;
-  apr_pool_t *scratch_pool = svn_pool_create(result_pool);
+  svn_fs_x__data_t *ffd = context->fs->fsap_data;
 
   svn_fs_x__id_t id;
-  id.change_set = svn_fs_x__change_set_by_rev(rev);
+  id.change_set = svn_fs_x__change_set_by_rev(context->revision);
   id.number = SVN_FS_X__ITEM_INDEX_CHANGES;
 
-  /* Provide revision file. */
-
-  SVN_ERR(svn_fs_x__ensure_revision_exists(rev, fs, scratch_pool));
-  SVN_ERR(svn_fs_x__rev_file_init(&revision_file, fs, rev, scratch_pool));
-
   /* try cache lookup first */
 
-  if (svn_fs_x__is_packed_rev(fs, rev))
+  if (svn_fs_x__is_packed_rev(context->fs, context->revision))
     {
       apr_off_t offset;
       apr_uint32_t sub_item;
       svn_fs_x__pair_cache_key_t key;
 
-      SVN_ERR(svn_fs_x__item_offset(&offset, &sub_item, fs, revision_file,
+      SVN_ERR(svn_fs_x__item_offset(&offset, &sub_item, context->fs,
+                                    context->revision_file,
                                     &id, scratch_pool));
-      key.revision = svn_fs_x__packed_base_rev(fs, rev);
+      key.revision = svn_fs_x__packed_base_rev(context->fs,
+                                               context->revision);
       key.second = offset;
 
       SVN_ERR(svn_cache__get_partial((void **)changes, &found,
@@ -2844,22 +2857,22 @@ svn_fs_x__get_changes(apr_array_header_t
   else
     {
       SVN_ERR(svn_cache__get((void **) changes, &found, ffd->changes_cache,
-                             &rev, result_pool));
+                             &context->revision, result_pool));
     }
 
   if (!found)
     {
       /* 'block-read' will also provide us with the desired data */
-      SVN_ERR(block_read((void **)changes, fs, &id, revision_file,
-                         result_pool, scratch_pool));
-
-      SVN_ERR(svn_fs_x__close_revision_file(revision_file));
+      SVN_ERR(block_read((void **)changes, context->fs, &id,
+                         context->revision_file, result_pool, scratch_pool));
     }
 
-  SVN_ERR(dgb__log_access(fs, &id, *changes, SVN_FS_X__ITEM_TYPE_CHANGES,
-                          scratch_pool));
+  context->next += (*changes)->nelts;
+  context->eol = TRUE;
+
+  SVN_ERR(dgb__log_access(context->fs, &id, *changes,
+                          SVN_FS_X__ITEM_TYPE_CHANGES, scratch_pool));
 
-  svn_pool_destroy(scratch_pool);
   return SVN_NO_ERROR;
 }
 

Modified: subversion/trunk/subversion/libsvn_fs_x/cached_data.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/cached_data.h?rev=1733804&r1=1733803&r2=1733804&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/cached_data.h (original)
+++ subversion/trunk/subversion/libsvn_fs_x/cached_data.h Sun Mar  6 12:21:50 2016
@@ -168,13 +168,24 @@ svn_fs_x__get_proplist(apr_hash_t **prop
                        apr_pool_t *result_pool,
                        apr_pool_t *scratch_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.
+ * Use SCRATCH_POOL for temporary allocations. */
+svn_error_t *
+svn_fs_x__create_changes_context(svn_fs_x__changes_context_t **context,
+                                 svn_fs_t *fs,
+                                 svn_revnum_t rev,
+                                 apr_pool_t *result_pool,
+                                 apr_pool_t *scratch_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_x__get_changes(apr_array_header_t **changes,
-                      svn_fs_t *fs,
-                      svn_revnum_t rev,
-                      apr_pool_t *pool);
+                      svn_fs_x__changes_context_t *context,
+                      apr_pool_t *result_pool,
+                      apr_pool_t *scratch_pool);
 
 #endif

Modified: subversion/trunk/subversion/libsvn_fs_x/fs.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/fs.h?rev=1733804&r1=1733803&r2=1733804&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/fs.h (original)
+++ subversion/trunk/subversion/libsvn_fs_x/fs.h Sun Mar  6 12:21:50 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" {
@@ -523,6 +523,26 @@ typedef struct svn_fs_x__dirent_t
 /*** Change ***/
 typedef svn_fs_path_change3_t svn_fs_x__change_t;
 
+/*** Context for reading changed paths lists iteratively. */
+typedef struct svn_fs_x__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. */
+  svn_fs_x__revision_file_t *revision_file;
+
+  /* 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_x__changes_context_t;
+
 /*** Directory (only used at the cache interface) ***/
 typedef struct svn_fs_x__dir_data_t
 {

Modified: subversion/trunk/subversion/libsvn_fs_x/tree.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/tree.c?rev=1733804&r1=1733803&r2=1733804&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/tree.c (original)
+++ subversion/trunk/subversion/libsvn_fs_x/tree.c Sun Mar  6 12:21:50 2016
@@ -2266,11 +2266,19 @@ 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_x__changes_context_t *context;
+
   /* Changes to send. */
   apr_array_header_t *changes;
 
   /* Current indexes within CHANGES. */
   int idx;
+
+  /* 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;
 
 static svn_error_t *
@@ -2279,6 +2287,21 @@ x_revision_changes_iterator_get(svn_fs_p
 {
   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_x__get_changes(&data->changes, data->context,
+                                    changes_pool, data->scratch_pool));
+
+      /* Immediately release any temporary data. */
+      svn_pool_clear(data->scratch_pool);
+    }
+
   if (data->idx < data->changes->nelts)
     {
       *change = APR_ARRAY_IDX(data->changes, data->idx,
@@ -2318,11 +2341,27 @@ x_report_changes(svn_fs_path_change_iter
     }
   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_x__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_x__create_changes_context(&data->context,
+                                               root->fs, root->rev,
+                                               result_pool, scratch_pool));
+      SVN_ERR(svn_fs_x__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;
     }