Skip to content

Commit

Permalink
Change semantics of put method for LRUCache objects
Browse files Browse the repository at this point in the history
This changes the `LRUCache.put` method to provide a general
way to insert an object into the cache.  Previously,
`LRUCache.put` wasn't sufficient for this as it appeared to only
behave as a "touch" method (e.g. if the item wasn't already
in the cache, it was a noop).

This is useful to provide a way to "seed" the various caches.
In a number of locations we construct items and put them in
the DB, only to immediately load them back.  We already have
caches around these DB accesses to avoid multiple "gets"
actually hitting the DB, but we can avoid the initial "get"
missing in many cases by seeding the associated cache when
we put the item into the DB.

An example of this is chrdicts and the chrdict cache, where
it's common to call `addChange` followed by `getChange`.
This results in two DB accesses when it really only needs to
be one.
  • Loading branch information
andrewjcg committed Apr 7, 2014
1 parent 2d4cbba commit 381a202
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 10 deletions.
3 changes: 3 additions & 0 deletions master/buildbot/test/fake/fakemaster.py
Expand Up @@ -46,6 +46,9 @@ def mkref(x):
d.addCallback(mkref)
return d

def put(self, key, val):
pass


class FakeCaches(object):

Expand Down
2 changes: 1 addition & 1 deletion master/buildbot/test/unit/test_util_lru.py
Expand Up @@ -252,7 +252,7 @@ def test_put_nonexistent_key(self):
self.assertEqual(self.lru.get('p'), short('p'))
self.lru.put('q', set(['new-q']))
self.assertEqual(self.lru.get('p'), set(['PPP']))
self.assertEqual(self.lru.get('q'), set(['QQQ'])) # not updated
self.assertEqual(self.lru.get('q'), set(['new-q'])) # updated


class AsyncLRUCacheTest(unittest.TestCase):
Expand Down
11 changes: 6 additions & 5 deletions master/buildbot/util/lru.py
Expand Up @@ -45,11 +45,12 @@ def __init__(self, miss_fn, max_size=50):
self.miss_fn = miss_fn

def put(self, key, value):
if key in self.cache:
self.cache[key] = value
self.weakrefs[key] = value
elif key in self.weakrefs:
self.weakrefs[key] = value
cached = key in self.cache or key in self.weakrefs
self.cache[key] = value
self.weakrefs[key] = value
self._ref_key(key)
if not cached:
self._purge()

def get(self, key, **miss_fn_kwargs):
try:
Expand Down
7 changes: 3 additions & 4 deletions master/docs/developer/utils.rst
Expand Up @@ -220,10 +220,9 @@ buildbot.util.lru
:param key: key at which to place the value
:param value: value to place there

Update the cache with the given key and value, but only if the key is
already in the cache. The purpose of this method is to insert a new
value into the cache *without* invoking the miss_fn (e.g., to avoid
unnecessary overhead).
Add the given key and value into the cache. The purpose of this
method is to insert a new value into the cache *without* invoking
the miss_fn (e.g., to avoid unnecessary overhead).

.. py:method set_max_size(max_size)
Expand Down

0 comments on commit 381a202

Please sign in to comment.