Mailing-List: contact commits-help@subversion.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@subversion.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1795324 - /subversion/trunk/subversion/libsvn_fs_fs/revprops.c
Date: Tue, 16 May 2017 14:59:12 -0000
To: commits@subversion.apache.org
From: stefan2@apache.org
Message-Id: <20170516145912.4CBE33A0332@svn01-us-west.apache.org>
archived-at: Tue, 16 May 2017 14:59:16 -0000

Author: stefan2
Date: Tue May 16 14:59:12 2017
New Revision: 1795324

URL: http://svn.apache.org/viewvc?rev=1795324&view=rev
Log:
Optimize the FSFS revprop cache filling strategy under high load.

With high eviction rates, we may end up writing all revprops in a
pack to the cache over and over again with hardly hitting any of
them in lookups.  This patch will identify thoses cases by the
large percentage of revprops already in cache for a given pack
while still we encountered a miss.

* subversion/libsvn_fs_fs/revprops.c
  (cache_revprops): Optionally, return whether an insertion was needed.
  (read_non_packed_revprop): Update caller.
  (parse_packed_revprops): Implement the heuristics to stop populating
                           the cache when it becomes ineffective.

Modified:
    subversion/trunk/subversion/libsvn_fs_fs/revprops.c

Modified: subversion/trunk/subversion/libsvn_fs_fs/revprops.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/revprops.c?rev=1795324&r1=1795323&r2=1795324&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/revprops.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/revprops.c Tue May 16 14:59:12 2017
@@ -229,9 +229,12 @@ prepare_revprop_cache(svn_fs_t *fs,
 }
 
 /* Store the unparsed revprop hash CONTENT for REVISION in FS's revprop
- * cache.  Use SCRATCH_POOL for temporary allocations. */
+ * cache.  If CACHED is not NULL, set *CACHED if there already is such
+ * an entry and skip the cache write in that case.  Use SCRATCH_POOL for
+ * temporary allocations. */
 static svn_error_t *
-cache_revprops(svn_fs_t *fs,
+cache_revprops(svn_boolean_t *is_cached,
+               svn_fs_t *fs,
                svn_revnum_t revision,
                svn_string_t *content,
                apr_pool_t *scratch_pool)
@@ -244,6 +247,14 @@ cache_revprops(svn_fs_t *fs,
   key.revision = revision;
   key.second = ffd->revprop_prefix;
 
+  if (is_cached)
+    {
+      SVN_ERR(svn_cache__has_key(is_cached, ffd->revprop_cache, &key,
+                                 scratch_pool));
+      if (*is_cached)
+        return SVN_NO_ERROR;
+    }
+
   SVN_ERR(svn_cache__set(ffd->revprop_cache, &key, content, scratch_pool));
 
   return SVN_NO_ERROR;
@@ -287,7 +298,7 @@ read_non_packed_revprop(apr_hash_t **pro
       SVN_ERR(parse_revprop(properties, fs, rev, as_string, pool, iterpool));
 
       if (populate_cache)
-        SVN_ERR(cache_revprops(fs, rev, as_string, iterpool));
+        SVN_ERR(cache_revprops(NULL, fs, rev, as_string, iterpool));
     }
 
   svn_pool_clear(iterpool);
@@ -453,6 +464,9 @@ parse_packed_revprops(svn_fs_t *fs,
   const char *header_end;
   apr_pool_t *iterpool = svn_pool_create(scratch_pool);
 
+  /* Initial value for the "Leaking bucket" pattern. */
+  int bucket = 4;
+
   /* decompress (even if the data is only "stored", there is still a
    * length header to remove) */
   svn_stringbuf_t *compressed = revprops->packed_revprops;
@@ -546,7 +560,30 @@ parse_packed_revprops(svn_fs_t *fs,
         }
 
       if (populate_cache)
-        SVN_ERR(cache_revprops(fs, revision, &serialized, iterpool));
+        {
+          /* Adding all those revprops is expensive, in particular in a
+           * multi-threaded environment.  There are situations where hit
+           * rates are low and revprops get evicted before re-using them.
+           *
+           * We try to detect thosse cases here.
+           * Only keep going while most (at least 2/3) aren't cached, yet. */
+          svn_boolean_t already_cached;
+          SVN_ERR(cache_revprops(&already_cached, fs, revision, &serialized,
+                                 iterpool));
+
+          /* Stop populating the cache once we encountered too many entries
+           * already present relative to the numbers being added. */
+          if (!already_cached)
+            {
+              ++bucket;
+            }
+          else
+            {
+              bucket -= 2;
+              if (bucket < 0)
+                populate_cache = FALSE;
+            }
+        }
 
       if (read_all)
         {