[Monotone-devel] Re: times to load various things from the database

[Derek Scherger]

On Sat, Jan 10, 2009 at 11:58 PM, Derek Scherger <address@hidden> wrote:

Sat Jan 10 09:24 PM ~/monotone/mainline $ time ./mtn db load_rosters
mtn: loading rosters
mtn:       rosters
mtn: 14,499/14,499

real    25m11.219s
user    24m20.875s
sys     0m44.263s

The following patch improves this by a factor of 3:

Sat Jan 17 09:47 PM ~/monotone/mainline $ time ./mtn db load_rosters
mtn: loading rosters
mtn:       rosters
mtn: 14,541/14,541

real    7m55.062s
user    7m46.029s
sys     0m9.061s

This patch does three things:

- every time a base roster (i.e. fulltext) is loaded, it is inserted into the roster cache. this seems generally useful as every roster reconstruction will require a base roster to start from. loading a 350KB roster seems to take about 30ms here, grabbing one from the cache is around 3ms.

- after a reconstruction path is loaded it is scanned in forward order looking for a cached roster to start from. if one is found, the reconstruction path is truncated at that point and reconstruction will start from the closest cached roster.

- when applying a chain of deltas to a base roster, every 64th roster is cached so that the previous step is likely to hit something.

I'm not sure if this is likely to cause problems with other aspects of performance or not. I'll try timing the testsuite, a full pull and maybe a db check as well with and without to see what it does.

I also wonder whether our cache sizes should be configured via lua hooks rather than header file constants. A 7MB static cache size doesn't seem very generous, sitting here on a an older system with 2GB of ram. I don't see anything in the roster cache stuff to report on sizes, hit rates, etc. so evaluating the cache performance would require more work.

Cheers,
Derek

#
# old_revision [cef7a2d9c06e0b96648cce7980c5e700b93f5add]
#
# patch "database.cc"
#  from [1d015db8c66b3d39c37b2bb7b759f5b21854a6b8]
#    to [3fe17463fadcd76df9fcc1d45b6a7b698e2d0bf8]
#
============================================================
--- database.cc 1d015db8c66b3d39c37b2bb7b759f5b21854a6b8
+++ database.cc 3fe17463fadcd76df9fcc1d45b6a7b698e2d0bf8
@@ -1620,6 +1620,17 @@ database_impl::get_roster_base(revision_
   data dat;
   decode_gzip(dat_packed, dat);
   read_roster_and_marking(roster_data(dat), roster, marking);
+
+  // cache this base roster
+
+  shared_ptr<roster_t> r(new roster_t);
+  shared_ptr<marking_map> m(new marking_map);
+  cached_roster cr;
+  *r = roster;
+  *m = marking;
+  cr.first = r;
+  cr.second = m;
+  roster_cache.insert_clean(ident, cr);
 }
 
 void
@@ -1988,6 +1999,15 @@ database::get_roster_version(revision_id
     get_reconstruction_path(ros_id.inner(), graph, selected_path);
   }
 
+  // scan the reconstruction path in forward order looking for a cache hit
+  // if we find one, truncate the reconstruction path at that point
+  for (size_t i = 0; i < selected_path.size(); ++i)
+    if (imp->roster_cache.exists(revision_id(selected_path[i]))) 
+      {
+        selected_path.resize(i+1);
+        break;
+      }
+
   id curr(selected_path.back());
   selected_path.pop_back();
   // we know that this isn't already in the cache (because of the early exit
@@ -1996,6 +2016,7 @@ database::get_roster_version(revision_id
   shared_ptr<marking_map> marking(new marking_map);
   imp->get_roster_base(revision_id(curr), *roster, *marking);
 
+  int cache_count = 0;
   for (reconstruction_path::reverse_iterator i = selected_path.rbegin();
        i != selected_path.rend(); ++i)
     {
@@ -2006,6 +2027,20 @@ database::get_roster_version(revision_id
       imp->get_roster_delta(nxt, curr, del);
       apply_roster_delta(del, *roster, *marking);
       curr = nxt;
+
+      if (++cache_count % 64 == 0 && !imp->roster_cache.exists(revision_id(*i))) 
+        {
+          // insert the current roster into the cache
+          shared_ptr<roster_t> r(new roster_t);
+          shared_ptr<marking_map> m(new marking_map);
+          *r = *roster;
+          *m = *marking;
+          cached_roster cr;
+          cr.first = r;
+          cr.second = m;
+          imp->roster_cache.insert_clean(revision_id(*i), cr);
+        }
+
     }
 
   // Double-check that the thing we got out looks okay.  We know that when
@@ -2027,7 +2062,8 @@ database::get_roster_version(revision_id
   // const'ify the objects, to save them and pass them out
   cr.first = roster;
   cr.second = marking;
-  imp->roster_cache.insert_clean(ros_id, cr);
+  if (!imp->roster_cache.exists(ros_id))
+    imp->roster_cache.insert_clean(ros_id, cr);
 }