subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From stef...@apache.org
Subject svn commit: r1744987 - in /subversion/trunk/subversion/libsvn_fs_x: cached_data.c caching.c fs.h temp_serializer.c temp_serializer.h
Date Sat, 21 May 2016 21:19:28 GMT
Author: stefan2
Date: Sat May 21 21:19:28 2016
New Revision: 1744987

URL: http://svn.apache.org/viewvc?rev=1744987&view=rev
Log:
In FSX, complete the chunked read support for FSFS-style changed paths lists.

This patch does two things.  Instead of full changes lists, it caches only
blocks of up to 100 entires - possibly more than one per revision.  Secondly,
it needs to be able to seek to specific blocks of changes if they should not
be found in the cache.  So, we need to remember their offsets within the
on-disk representation.   OTOH, we can remove some transitional code.

* subversion/libsvn_fs_x/fs.h
  (svn_fs_x__data_t.changes_cache):  Update docstring.
  (svn_fs_x__changes_context): Add the NEXT_OFFSET element, so we know where
                               to read from disk when we need to fall back
                               to that.

* subversion/libsvn_fs_x/temp_serializer.h
  (svn_fs_x__changes_list_t):  New data structure that decorated the plain
                               APR array of changes with per-block info such
                               as relative position on disk etc.
  (svn_fs_x__serialize_changes,
   svn_fs_x__deserialize_changes): Update docstrings as we expect the changes
                                   list to be of the above type now.
  (svn_fs_x__read_changes_block_baton_t,
   svn_fs_x__read_changes_block): We don't need partial data access anymore
                                  because we cache on block granularity now.

* subversion/libsvn_fs_x/temp_serializer.c
  (changes_data_t):  Superseeded by svn_fs_x__changes_list_t.
  (svn_fs_x__serialize_changes,
   svn_fs_x__deserialize_changes):  Update.  The new data type is similar
                                    to the one used before internally.
  (svn_fs_x__read_changes_block): Remove obsolete function.

* subversion/libsvn_fs_x/caching.c
  (svn_fs_x__initialize_caches):  FSFS-style changed paths lists are now
                                  keyed by rev,block pairs.

* subversion/libsvn_fs_x/cached_data.c
  (svn_fs_x__get_changes):  Instead of partial access to the full list, we
                            now have full access to the block we want next.
                            Update the iterator info accordingly.
  (block_read_changes): Verify the list contents only once per list.  Read
                        and cache only the requested block, otherwise.

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/temp_serializer.c
    subversion/trunk/subversion/libsvn_fs_x/temp_serializer.h

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=1744987&r1=1744986&r2=1744987&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/cached_data.c (original)
+++ subversion/trunk/subversion/libsvn_fs_x/cached_data.c Sat May 21 21:19:28 2016
@@ -2861,14 +2861,26 @@ svn_fs_x__get_changes(apr_array_header_t
     }
   else
     {
-      svn_fs_x__read_changes_block_baton_t baton;
-      baton.start = (int)context->next;
-      baton.eol = &context->eol;
-
-      SVN_ERR(svn_cache__get_partial((void **)changes, &found,
-                                     ffd->changes_cache, &context->revision,
-                                     svn_fs_x__read_changes_block,
-                                     &baton, result_pool));
+      svn_fs_x__changes_list_t *changes_list;
+      svn_fs_x__pair_cache_key_t key;
+      key.revision = context->revision;
+      key.second = context->next;
+
+      SVN_ERR(svn_cache__get((void **)&changes_list, &found,
+                             ffd->changes_cache, &key, result_pool));
+
+      if (found)
+        {
+          /* Where to look next - if there is more data. */
+          context->eol = changes_list->eol;
+          context->next_offset = changes_list->end_offset;
+
+          /* Return the block as a "proper" APR array. */
+          (*changes) = apr_array_make(result_pool, 0, sizeof(void *));
+          (*changes)->elts = (char *)changes_list->changes;
+          (*changes)->nelts = changes_list->count;
+          (*changes)->nalloc = changes_list->count;
+        }
     }
 
   if (!found)
@@ -2980,61 +2992,68 @@ block_read_changes(apr_array_header_t **
                    apr_pool_t *result_pool,
                    apr_pool_t *scratch_pool)
 {
+  enum { BLOCK_SIZE = 100 };
+
   svn_fs_x__data_t *ffd = fs->fsap_data;
   svn_stream_t *stream;
-  svn_revnum_t revision = svn_fs_x__get_revnum(entry->items[0].change_set);
-  apr_size_t estimated_size;
+  svn_fs_x__pair_cache_key_t key;
+  svn_fs_x__changes_list_t changes_list;
+
+  /* If we don't have to return any data, just read and cache the first
+     block.  This means we won't cache the remaining blocks from longer
+     lists right away but only if they are actually needed. */
+  apr_size_t next = must_read ? context->next : 0;
+  apr_size_t next_offset = must_read ? context->next_offset : 0;
 
   /* we don't support containers, yet */
   SVN_ERR_ASSERT(entry->item_count == 1);
 
+  /* The item to read / write. */
+  key.revision = svn_fs_x__get_revnum(entry->items[0].change_set);
+  key.second = next;
+
   /* already in cache? */
   if (!must_read)
     {
       svn_boolean_t is_cached = FALSE;
-      SVN_ERR(svn_cache__has_key(&is_cached, ffd->changes_cache, &revision,
+      SVN_ERR(svn_cache__has_key(&is_cached, ffd->changes_cache, &key,
                                  scratch_pool));
       if (is_cached)
         return SVN_NO_ERROR;
     }
 
-  SVN_ERR(read_item(&stream, fs, rev_file, entry, scratch_pool));
+  /* Verify the whole list only once.  We don't use the STREAM any further. */
+  if (!must_read || next == 0)
+    SVN_ERR(read_item(&stream, fs, rev_file, entry, scratch_pool));
+
+  /* Seek to the block to read within the changes list. */
+  SVN_ERR(svn_fs_x__rev_file_seek(rev_file, NULL,
+                                  entry->offset + next_offset));
+  SVN_ERR(svn_fs_x__rev_file_stream(&stream, rev_file));
 
   /* read changes from revision file */
-  SVN_ERR(svn_fs_x__read_changes(changes, stream, INT_MAX,
+  SVN_ERR(svn_fs_x__read_changes(changes, stream, BLOCK_SIZE,
                                  result_pool, scratch_pool));
 
-  /* cache for future reference */
+  SVN_ERR(svn_fs_x__rev_file_offset(&changes_list.end_offset, rev_file));
+  changes_list.end_offset -= entry->offset;
+  changes_list.start_offset = next_offset;
+  changes_list.count = (*changes)->nelts;
+  changes_list.changes = (svn_fs_x__change_t **)(*changes)->elts;
+  changes_list.eol =    (changes_list.count < BLOCK_SIZE)
+                     || (changes_list.end_offset + 1 >= entry->size);
 
-  /* Guesstimate for the size of the in-cache representation. */
-  estimated_size = (apr_size_t)250 * (*changes)->nelts;
+  /* cache for future reference */
 
-  /* Don't even serialize data that probably won't fit into the
-   * cache.  This often implies that either CHANGES is very
-   * 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, &revision, *changes,
-                           scratch_pool));
+  SVN_ERR(svn_cache__set(ffd->changes_cache, &key, &changes_list,
+                         scratch_pool));
 
   /* Trim the result:
    * Remove the entries that already been reported. */
-
-  /* TODO: This is transitional code.
-   *       The final implementation will read and cache only sections of
-   *       the changes list. */
   if (must_read)
     {
-      int i;
-      for (i = 0; i + context->next < (*changes)->nelts; ++i)
-        {
-          APR_ARRAY_IDX(*changes, i, svn_fs_x__change_t *)
-            = APR_ARRAY_IDX(*changes, i + context->next,
-                            svn_fs_x__change_t *);
-        }
-      (*changes)->nelts = i;
-
-      context->eol = TRUE;
+      context->next_offset = changes_list.end_offset;
+      context->eol = changes_list.eol;
     }
 
   return SVN_NO_ERROR;

Modified: subversion/trunk/subversion/libsvn_fs_x/caching.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/caching.c?rev=1744987&r1=1744986&r2=1744987&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/caching.c (original)
+++ subversion/trunk/subversion/libsvn_fs_x/caching.c Sat May 21 21:19:28 2016
@@ -487,7 +487,7 @@ svn_fs_x__initialize_caches(svn_fs_t *fs
                        1, 8, /* 1k / entry; 8 entries total, rarely used */
                        svn_fs_x__serialize_changes,
                        svn_fs_x__deserialize_changes,
-                       sizeof(svn_revnum_t),
+                       sizeof(svn_fs_x__pair_cache_key_t),
                        apr_pstrcat(scratch_pool, prefix, "CHANGES",
                                    SVN_VA_NULL),
                        0,

Modified: subversion/trunk/subversion/libsvn_fs_x/fs.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/fs.h?rev=1744987&r1=1744986&r2=1744987&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/fs.h (original)
+++ subversion/trunk/subversion/libsvn_fs_x/fs.h Sat May 21 21:19:28 2016
@@ -319,8 +319,8 @@ typedef struct svn_fs_x__data_t
      the key is a (pack file revision, file offset) pair */
   svn_cache__t *noderevs_container_cache;
 
-  /* Cache for change lists as APR arrays of svn_fs_x__change_t * objects;
-     the key is the revision */
+  /* Cache for change lists n blocks as svn_fs_x__changes_list_t * objects;
+     the key is the (revision, first-element-in-block) pair. */
   svn_cache__t *changes_cache;
 
   /* Cache for change_list_t containers;
@@ -538,6 +538,10 @@ typedef struct svn_fs_x__changes_context
   /* Index of the next change to fetch. */
   apr_size_t next;
 
+  /* Offset, within the changed paths list on disk, of the next change to
+     fetch. */
+  apr_off_t next_offset;
+
   /* Has the end of the list been reached? */
   svn_boolean_t eol;
 

Modified: subversion/trunk/subversion/libsvn_fs_x/temp_serializer.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/temp_serializer.c?rev=1744987&r1=1744986&r2=1744987&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/temp_serializer.c (original)
+++ subversion/trunk/subversion/libsvn_fs_x/temp_serializer.c Sat May 21 21:19:28 2016
@@ -1126,47 +1126,29 @@ deserialize_change(void *buffer,
   svn_temp_deserializer__resolve(change, (void **)&change->copyfrom_path);
 }
 
-/* Auxiliary structure representing the content of a svn_fs_x__change_t array.
-   This structure is much easier to (de-)serialize than an APR array.
- */
-typedef struct changes_data_t
-{
-  /* number of entries in the array */
-  int count;
-
-  /* reference to the changes */
-  svn_fs_x__change_t **changes;
-} changes_data_t;
-
 svn_error_t *
 svn_fs_x__serialize_changes(void **data,
                             apr_size_t *data_len,
                             void *in,
                             apr_pool_t *pool)
 {
-  apr_array_header_t *array = in;
-  changes_data_t changes;
+  svn_fs_x__changes_list_t *changes = in;
   svn_temp_serializer__context_t *context;
   svn_stringbuf_t *serialized;
   int i;
 
-  /* initialize our auxiliary data structure and link it to the
-   * array elements */
-  changes.count = array->nelts;
-  changes.changes = (svn_fs_x__change_t **)array->elts;
-
   /* serialize it and all its elements */
-  context = svn_temp_serializer__init(&changes,
-                                      sizeof(changes),
-                                      changes.count * 250,
+  context = svn_temp_serializer__init(changes,
+                                      sizeof(*changes),
+                                      changes->count * 250,
                                       pool);
 
   svn_temp_serializer__push(context,
-                            (const void * const *)&changes.changes,
-                            changes.count * sizeof(*changes.changes));
+                            (const void * const *)&changes->changes,
+                            changes->count * sizeof(*changes->changes));
 
-  for (i = 0; i < changes.count; ++i)
-    serialize_change(context, &changes.changes[i]);
+  for (i = 0; i < changes->count; ++i)
+    serialize_change(context, &changes->changes[i]);
 
   svn_temp_serializer__pop(context);
 
@@ -1186,9 +1168,7 @@ svn_fs_x__deserialize_changes(void **out
                               apr_pool_t *result_pool)
 {
   int i;
-  changes_data_t *changes = (changes_data_t *)data;
-  apr_array_header_t *array = apr_array_make(result_pool, 0,
-                                             sizeof(svn_fs_x__change_t *));
+  svn_fs_x__changes_list_t *changes = (svn_fs_x__changes_list_t *)data;
 
   /* de-serialize our auxiliary data structure */
   svn_temp_deserializer__resolve(changes, (void**)&changes->changes);
@@ -1198,69 +1178,8 @@ svn_fs_x__deserialize_changes(void **out
     deserialize_change(changes->changes,
                        (svn_fs_x__change_t **)&changes->changes[i]);
 
-  /* Use the changes buffer as the array's data buffer
-   * (DATA remains valid for at least as long as POOL). */
-  array->elts = (char *)changes->changes;
-  array->nelts = changes->count;
-  array->nalloc = changes->count;
-
-  /* done */
-  *out = array;
-
-  return SVN_NO_ERROR;
-}
-
-svn_error_t *
-svn_fs_x__read_changes_block(void **out,
-                             const void *data,
-                             apr_size_t data_len,
-                             void *baton,
-                             apr_pool_t *pool)
-{
-  int first;
-  int last;
-  int i;
-  enum { BLOCK_SIZE = 100 };
-  apr_array_header_t *array;
-
-  svn_fs_x__read_changes_block_baton_t *b = baton;
-  changes_data_t changes = *(const changes_data_t *)data;
-
-  /* Restrict range to the block requested by the BATON.
-   * Tell the caller whether we reached the end of the list. */
-  first = MIN(b->start, changes.count);
-  last = MIN(first + BLOCK_SIZE, changes.count);
-  *b->eol = last == changes.count;
-
-  /* de-serialize our auxiliary data structure */
-  svn_temp_deserializer__resolve(data, (void**)&changes.changes);
-
-  /* de-serialize each entry and add it to the array */
-  array = apr_array_make(pool, last - first, sizeof(svn_fs_x__change_t *));
-  for (i = first; i < last; ++i)
-    {
-      svn_fs_x__change_t *change;
-
-      /* Get a pointer to the in-cache change struct at offset I. */
-      svn_fs_x__change_t *cached_change = changes.changes[i];
-      svn_temp_deserializer__resolve(changes.changes,
-                                     (void**)&cached_change);
-
-      /* Duplicate that struct into the result POOL. */
-      change = apr_pmemdup(pool, cached_change, sizeof(*change));
-
-      /* fix-up of pointers within the struct */
-      svn_temp_deserializer__resolve(cached_change,
-                                     (void **)&change->path.data);
-      svn_temp_deserializer__resolve(cached_change,
-                                     (void **)&change->copyfrom_path);
-
-      /* Add the change to result. */
-      APR_ARRAY_PUSH(array, svn_fs_x__change_t *) = change;
-    }
-
   /* done */
-  *out = array;
+  *out = changes;
 
   return SVN_NO_ERROR;
 }

Modified: subversion/trunk/subversion/libsvn_fs_x/temp_serializer.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/temp_serializer.h?rev=1744987&r1=1744986&r2=1744987&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/temp_serializer.h (original)
+++ subversion/trunk/subversion/libsvn_fs_x/temp_serializer.h Sat May 21 21:19:28 2016
@@ -266,9 +266,34 @@ svn_fs_x__deserialize_rep_header(void **
                                  apr_size_t data_len,
                                  apr_pool_t *result_pool);
 
+/*** Block of changes in a changed paths list. */
+typedef struct svn_fs_x__changes_list_t
+{
+  /* Offset of the first element in CHANGES within the changed paths list
+     on disk. */
+  apr_off_t start_offset;
+
+  /* Offset of the first element behind CHANGES within the changed paths
+     list on disk. */
+  apr_off_t end_offset;
+
+  /* End of list reached? This may have false negatives in case the number
+     of elements in the list is a multiple of our block / range size. */
+  svn_boolean_t eol;
+
+  /* Array of #svn_fs_x__change_t * representing a consecutive sub-range of
+     elements in a changed paths list. */
+
+  /* number of entries in the array */
+  int count;
+
+  /* reference to the changes */
+  svn_fs_x__change_t **changes;
+
+} svn_fs_x__changes_list_t;
+
 /**
- * Implements #svn_cache__serialize_func_t for an #apr_array_header_t of
- * #svn_fs_x__change_t *.
+ * Implements #svn_cache__serialize_func_t for a #svn_fs_x__changes_list_t.
  */
 svn_error_t *
 svn_fs_x__serialize_changes(void **data,
@@ -277,8 +302,7 @@ svn_fs_x__serialize_changes(void **data,
                             apr_pool_t *pool);
 
 /**
- * Implements #svn_cache__deserialize_func_t for an #apr_array_header_t of
- * #svn_fs_x__change_t *.
+ * Implements #svn_cache__deserialize_func_t for a #svn_fs_x__changes_list_t.
  */
 svn_error_t *
 svn_fs_x__deserialize_changes(void **out,
@@ -286,28 +310,4 @@ svn_fs_x__deserialize_changes(void **out
                               apr_size_t data_len,
                               apr_pool_t *result_pool);
 
-/* Baton type to be used with svn_fs_x__read_changes_block. */
-typedef struct svn_fs_x__read_changes_block_baton_t
-{
-  /* Deliver data starting from this index within the changes list. */
-  int start;
-
-  /* To be set by svn_fs_x__read_changes_block:
-     Did we deliver the last change in that list? */
-  svn_boolean_t *eol;
-} svn_fs_x__read_changes_block_baton_t;
-
-/**
- * Implements #svn_cache__partial_getter_func_t, returning a number of
- * #svn_fs_x__change_t * in an #apr_array_header_t.  The @a *baton of type
- * 'svn_fs_x__read_changes_block_baton_t describes what the first index
- * in that block should be.
- */
-svn_error_t *
-svn_fs_x__read_changes_block(void **out,
-                             const void *data,
-                             apr_size_t data_len,
-                             void *baton,
-                             apr_pool_t *pool);
-
 #endif



Mime
View raw message