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/02/14 18:47:55 UTC

svn commit: r1730362 - in /subversion/trunk/subversion: include/svn_fs.h libsvn_fs/fs-loader.c libsvn_fs/fs-loader.h

Author: stefan2
Date: Sun Feb 14 17:47:55 2016
New Revision: 1730362

URL: http://svn.apache.org/viewvc?rev=1730362&view=rev
Log:
Switch the new svn_fs_paths_changed3 from a using callback to returning
an iterator object.

Suggested by: ivan

* subversion/include/svn_fs.h
  (svn_fs_path_change_iterator_t): Declare the new iterator object.
  (svn_fs_path_change_get): The only method our new iterator has.  To keep
                            it copy-free, we provide very limited lifetime
                            guarantees.  That's o.k. for such a low-level
                            API.
  (svn_fs_path_change_receiver_t): Drop callback type.
  (svn_fs_path_change3_create): Switch from callback to iterator.

* subversion/libsvn_fs/fs-loader.h
  (root_vtable_t): Update to match the changed API.
  (changes_iterator_vtable_t): V-table type for the new iterator object.
  (svn_fs_path_change_iterator_t): Define the generic iterator object.

* subversion/libsvn_fs/fs-loader.c
  (svn_fs_path_change_get): Implement new iterator API.
  (add_changed_path_baton_t,
   add_changed_path): Drop old emulation utilities.
  (svn_fs_paths_changed2): Update the emulation to the iterator-based API.
  (fsap_iterator_data_t,
   changes_iterator_get,
   iterator_vtable): Implement the new iterator for old API data.
  (svn_fs_paths_changed3): Update implementation.

Modified:
    subversion/trunk/subversion/include/svn_fs.h
    subversion/trunk/subversion/libsvn_fs/fs-loader.c
    subversion/trunk/subversion/libsvn_fs/fs-loader.h

Modified: subversion/trunk/subversion/include/svn_fs.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_fs.h?rev=1730362&r1=1730361&r2=1730362&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_fs.h (original)
+++ subversion/trunk/subversion/include/svn_fs.h Sun Feb 14 17:47:55 2016
@@ -1660,26 +1660,39 @@ svn_fs_path_change3_create(svn_fs_path_c
                            apr_pool_t *result_pool);
 
 /**
- * Processing callback type used with svn_fs_paths_changed3.
+ * Opaque iterator object type for a changed paths list.
  *
- * The @a baton is the baton pointer provided to svn_fs_paths_changed3
- * and @a change is the path change to process.  You may modify its
- * content and it will become invalid as soon as callback returns.
+ * @since New in 1.10.
+ */
+typedef struct svn_fs_path_change_iterator_t svn_fs_path_change_iterator_t;
+
+/**
+ * Set @a *change to the path change that @a iterator currently points to
+ * and advance the @a iterator.  If the change list has been exhausted,
+ * @a change will be set to @c NULL.
  *
- * Use @a scratch_pool for temporary allocations.
+ * You may modify @a **change but its content becomes invalid as soon as
+ * either @a iterator becomes invalid or you call this function again.
  *
  * @note The @c node_kind field in @a change may be #svn_node_unknown and
  *       the @c copyfrom_known fields may be FALSE.
+ *
+ * @since New in 1.10.
  */
-typedef svn_error_t *(*svn_fs_path_change_receiver_t)(
-  void *baton,
-  svn_fs_path_change3_t *change,
-  apr_pool_t *scratch_pool);
+svn_error_t *
+svn_fs_path_change_get(svn_fs_path_change3_t **change,
+                       svn_fs_path_change_iterator_t *iterator);
+
 
 /** Determine what has changed under a @a root.
  *
- * For each changed path, invoke @a receiver with @a baton and the
- * respective change.
+ * Set @a *iterator to an iterator object, allocated in @a result_pool,
+ * which will give access to the full list of changed paths under @a root.
+ * Each call to @a svn_fs_path_change_get will return a new unique path
+ * change.  The iteration order is undefined and may change even for the
+ * same @a root.
+ *
+ * If @a root becomes invalid, @a *iterator becomes invalid, too.
  *
  * Callers can assume that this function takes time proportional to
  * the amount of data output, and does not need to do tree crawls;
@@ -1689,12 +1702,16 @@ typedef svn_error_t *(*svn_fs_path_chang
  *
  * Use @a scratch_pool for temporary allocations.
  *
+ * @note The @a *iterator may be a large object and bind limited system
+ *       resources such as file handles.  Be sure to clear the owning
+ *       pool once you don't need that iterator anymore.
+ *
  * @since New in 1.10.
  */
 svn_error_t *
-svn_fs_paths_changed3(svn_fs_root_t *root,
-                      svn_fs_path_change_receiver_t receiver,
-                      void *baton,
+svn_fs_paths_changed3(svn_fs_path_change_iterator_t **iterator,
+                      svn_fs_root_t *root,
+                      apr_pool_t *result_pool,
                       apr_pool_t *scratch_pool);
 
 /** Determine what has changed under a @a root.

Modified: subversion/trunk/subversion/libsvn_fs/fs-loader.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs/fs-loader.c?rev=1730362&r1=1730361&r2=1730362&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs/fs-loader.c (original)
+++ subversion/trunk/subversion/libsvn_fs/fs-loader.c Sun Feb 14 17:47:55 2016
@@ -1055,53 +1055,11 @@ svn_fs_revision_root_revision(svn_fs_roo
   return root->is_txn_root ? SVN_INVALID_REVNUM : root->rev;
 }
 
-/* Baton type to be used with add_changed_path.
-   It contains extra parameters in and out of svn_fs_paths_changed2. */
-typedef struct add_changed_path_baton_t
-{
-  svn_fs_root_t *root;
-  apr_hash_t *changes;
-} add_changed_path_baton_t;
-
-/* Implements svn_fs_path_change_receiver_t.
-   Take the CHANGE, convert it into a svn_fs_path_change2_t and add
-   it to the CHANGES hash in the add_changed_path_baton_t BATON. */
-static svn_error_t *
-add_changed_path(void *baton,
-                 svn_fs_path_change3_t *change,
-                 apr_pool_t *scratch_pool)
-{
-  add_changed_path_baton_t *b = baton;
-  apr_pool_t *result_pool = apr_hash_pool_get(b->changes);
-
-  svn_fs_path_change2_t *copy;
-  const svn_fs_id_t *id_copy;
-
-  svn_fs_root_t *root = b->root;
-  const char *path = change->path.data;
-  if (change->change_kind == svn_fs_path_change_delete)
-    SVN_ERR(svn_fs__get_deleted_node(&root, &path, root, path,
-                                     scratch_pool, scratch_pool));
-
-  SVN_ERR(svn_fs_node_id(&id_copy, root, path, result_pool));
-
-  copy = svn_fs_path_change2_create(id_copy, change->change_kind,
-                                    result_pool);
-  copy->copyfrom_known = change->copyfrom_known;
-  if (copy->copyfrom_known && SVN_IS_VALID_REVNUM(change->copyfrom_rev))
-    {
-      copy->copyfrom_rev = change->copyfrom_rev;
-      copy->copyfrom_path = apr_pstrdup(result_pool, change->copyfrom_path);
-    }
-  copy->mergeinfo_mod = change->mergeinfo_mod;
-  copy->node_kind = change->node_kind;
-  copy->prop_mod = change->prop_mod;
-  copy->text_mod = change->text_mod;
-
-  svn_hash_sets(b->changes, apr_pstrmemdup(result_pool, change->path.data,
-                                           change->path.len), copy);
-
-  return SVN_NO_ERROR;
+svn_error_t *
+svn_fs_path_change_get(svn_fs_path_change3_t **change,
+                       svn_fs_path_change_iterator_t *iterator)
+{
+  return iterator->vtable->get(change, iterator);
 }
 
 svn_error_t *
@@ -1114,11 +1072,54 @@ svn_fs_paths_changed2(apr_hash_t **chang
 
   if (emulate)
     {
-      add_changed_path_baton_t baton;
-      baton.root = root;
-      baton.changes = svn_hash__make(pool);
-      SVN_ERR(svn_fs_paths_changed3(root, add_changed_path, &baton, pool));
-      *changed_paths_p = baton.changes;
+      apr_pool_t *scratch_pool = svn_pool_create(pool);
+      apr_hash_t *changes = svn_hash__make(pool);
+
+      svn_fs_path_change_iterator_t *iterator;
+      svn_fs_path_change3_t *change;
+
+      SVN_ERR(svn_fs_paths_changed3(&iterator, root, scratch_pool,
+                                    scratch_pool));
+
+      SVN_ERR(svn_fs_path_change_get(&change, iterator));
+      while (change)
+        {
+          svn_fs_path_change2_t *copy;
+          const svn_fs_id_t *id_copy;
+          const char *change_path = change->path.data;
+          svn_fs_root_t *change_root = root;
+
+          /* Copy CHANGE to old API struct. */
+          if (change->change_kind == svn_fs_path_change_delete)
+            SVN_ERR(svn_fs__get_deleted_node(&change_root, &change_path,
+                                             change_root, change_path,
+                                             scratch_pool, scratch_pool));
+
+          SVN_ERR(svn_fs_node_id(&id_copy, change_root, change_path, pool));
+
+          copy = svn_fs_path_change2_create(id_copy, change->change_kind,
+                                            pool);
+          copy->copyfrom_known = change->copyfrom_known;
+          if (   copy->copyfrom_known
+              && SVN_IS_VALID_REVNUM(change->copyfrom_rev))
+            {
+              copy->copyfrom_rev = change->copyfrom_rev;
+              copy->copyfrom_path = apr_pstrdup(pool, change->copyfrom_path);
+            }
+          copy->mergeinfo_mod = change->mergeinfo_mod;
+          copy->node_kind = change->node_kind;
+          copy->prop_mod = change->prop_mod;
+          copy->text_mod = change->text_mod;
+
+          svn_hash_sets(changes, apr_pstrmemdup(pool, change->path.data,
+                                                change->path.len), copy);
+
+          /* Next change. */
+          SVN_ERR(svn_fs_path_change_get(&change, iterator));
+        }
+      svn_pool_destroy(scratch_pool);
+
+      *changed_paths_p = changes;
     }
   else
     {
@@ -1128,10 +1129,61 @@ svn_fs_paths_changed2(apr_hash_t **chang
   return SVN_NO_ERROR;
 }
 
+/* Implement svn_fs_path_change_iterator_t on top of svn_fs_paths_changed2. */
+
+/* The iterator builds upon a hash iterator, which in turn operates on the
+   full prefetched changes list. */
+typedef struct fsap_iterator_data_t
+{
+  apr_hash_index_t *hi;
+
+  /* For efficicency such that we don't need to dynamically allocate
+     yet another copy of that data. */
+  svn_fs_path_change3_t change;
+} fsap_iterator_data_t;
+
+static svn_error_t *
+changes_iterator_get(svn_fs_path_change3_t **change,
+                     svn_fs_path_change_iterator_t *iterator)
+{
+  fsap_iterator_data_t *data = iterator->fsap_data;
+
+  if (data->hi)
+    {
+      const char *path = apr_hash_this_key(data->hi);
+      svn_fs_path_change2_t *entry = apr_hash_this_val(data->hi);
+
+      data->change.path.data = path;
+      data->change.path.len = apr_hash_this_key_len(data->hi);
+      data->change.change_kind = entry->change_kind;
+      data->change.node_kind = entry->node_kind;
+      data->change.text_mod = entry->text_mod;
+      data->change.prop_mod = entry->prop_mod;
+      data->change.mergeinfo_mod = entry->mergeinfo_mod;
+      data->change.copyfrom_known = entry->copyfrom_known;
+      data->change.copyfrom_rev = entry->copyfrom_rev;
+      data->change.copyfrom_path = entry->copyfrom_path;
+
+      *change = &data->change;
+      data->hi = apr_hash_next(data->hi);
+    }
+  else
+    {
+      *change = NULL;
+    }
+
+  return SVN_NO_ERROR;
+}
+
+static changes_iterator_vtable_t iterator_vtable =
+{
+  changes_iterator_get
+};
+
 svn_error_t *
-svn_fs_paths_changed3(svn_fs_root_t *root,
-                      svn_fs_path_change_receiver_t receiver,
-                      void *baton,
+svn_fs_paths_changed3(svn_fs_path_change_iterator_t **iterator,
+                      svn_fs_root_t *root,
+                      apr_pool_t *result_pool,
                       apr_pool_t *scratch_pool)
 {
   svn_boolean_t emulate =    !root->vtable->report_changes
@@ -1140,40 +1192,24 @@ svn_fs_paths_changed3(svn_fs_root_t *roo
 
   if (emulate)
     {
+      svn_fs_path_change_iterator_t *result;
+      fsap_iterator_data_t *data;
+
       apr_hash_t *changes;
-      apr_hash_index_t *hi;
-      apr_pool_t *iterpool = svn_pool_create(scratch_pool);
-      SVN_ERR(root->vtable->paths_changed(&changes, root, scratch_pool));
-
-      for (hi = apr_hash_first(scratch_pool, changes);
-           hi;
-           hi = apr_hash_next(hi))
-        {
-          const char *path = apr_hash_this_key(hi);
-          svn_fs_path_change2_t *change = apr_hash_this_val(hi);
-          svn_fs_path_change3_t path_change = { 0 };
-
-          svn_pool_clear(iterpool);
-
-          path_change.path.data = path;
-          path_change.path.len = apr_hash_this_key_len(hi);
-          path_change.change_kind = change->change_kind;
-          path_change.node_kind = change->node_kind;
-          path_change.text_mod = change->text_mod;
-          path_change.prop_mod = change->prop_mod;
-          path_change.mergeinfo_mod = change->mergeinfo_mod;
-          path_change.copyfrom_known = change->copyfrom_known;
-          path_change.copyfrom_rev = change->copyfrom_rev;
-          path_change.copyfrom_path = change->copyfrom_path;
+      SVN_ERR(root->vtable->paths_changed(&changes, root, result_pool));
 
-          SVN_ERR(receiver(baton, &path_change, iterpool));
-        }
+      data = apr_pcalloc(result_pool, sizeof(*data));
+      data->hi = apr_hash_first(result_pool, changes);
+
+      result = apr_pcalloc(result_pool, sizeof(*result));
+      result->fsap_data = data;
+      result->vtable = &iterator_vtable;
 
-      svn_pool_destroy(iterpool);
+      *iterator = result;
     }
   else
     {
-      SVN_ERR(root->vtable->report_changes(root, receiver, baton,
+      SVN_ERR(root->vtable->report_changes(iterator, root, result_pool,
                                            scratch_pool));
     }
 

Modified: subversion/trunk/subversion/libsvn_fs/fs-loader.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs/fs-loader.h?rev=1730362&r1=1730361&r2=1730362&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs/fs-loader.h (original)
+++ subversion/trunk/subversion/libsvn_fs/fs-loader.h Sun Feb 14 17:47:55 2016
@@ -301,9 +301,9 @@ typedef struct root_vtable_t
   svn_error_t *(*paths_changed)(apr_hash_t **changed_paths_p,
                                 svn_fs_root_t *root,
                                 apr_pool_t *pool);
-  svn_error_t *(*report_changes)(svn_fs_root_t *root,
-                                 svn_fs_path_change_receiver_t receiver,
-                                 void *baton,
+  svn_error_t *(*report_changes)(svn_fs_path_change_iterator_t **iterator,
+                                 svn_fs_root_t *root,
+                                 apr_pool_t *result_pool,
                                  apr_pool_t *scratch_pool);
 
   /* Generic node operations */
@@ -430,6 +430,13 @@ typedef struct root_vtable_t
 } root_vtable_t;
 
 
+typedef struct changes_iterator_vtable_t
+{
+  svn_error_t *(*get)(svn_fs_path_change3_t **change,
+                      svn_fs_path_change_iterator_t *iterator);
+} changes_iterator_vtable_t;
+
+
 typedef struct history_vtable_t
 {
   svn_error_t *(*prev)(svn_fs_history_t **prev_history_p,
@@ -532,6 +539,12 @@ struct svn_fs_root_t
   void *fsap_data;
 };
 
+struct svn_fs_path_change_iterator_t
+{
+  /* FSAP-specific vtable and private data */
+  const changes_iterator_vtable_t *vtable;
+  void *fsap_data;
+};
 
 struct svn_fs_history_t
 {