diff --git a/axiom/item.py b/axiom/item.py index ec821265..b232579c 100644 --- a/axiom/item.py +++ b/axiom/item.py @@ -192,6 +192,12 @@ class Empowered(object): are being loaded and with a list of powerups found in C{store}. The return value is the powerup. These are used only by the callable interface adaption API, not C{powerupsFor}. + + @type _dbPowerups: C{dict} + @ivar _dbPowerups: Mapping from interface classes to lists of (priority, + item). This is only populated on the first call to L{powerupsFor} for a + particular interface; thereafter, it will be updated when powerups are + added and removed, to avoid loading the powerups from the database again. """ aggregateInterfaces = { @@ -270,6 +276,11 @@ def powerUp(self, powerup, interface=None, priority=0): powerup=powerup) forc.priority = priority + pups = self._dbPowerups.get(interface) + if pups is not None: + pups.append((priority, powerup)) + pups.sort(key=lambda (prio, p): prio, reverse=True) + def powerDown(self, powerup, interface=None): """ @@ -298,6 +309,10 @@ def powerDown(self, powerup, interface=None): AND(_PowerupConnector.item == self, _PowerupConnector.interface == unicode(qual(interface)), _PowerupConnector.powerup == powerup)): + pups = self._dbPowerups.get(interface) + if pups is not None: + for elem in [elem for elem in pups if elem[1] is cable.powerup]: + pups.remove(elem) cable.deleteFromStore() return raise ValueError("Not powered up for %r with %r" % (interface, @@ -341,6 +356,27 @@ def powerupsFor(self, interface): inMemoryPowerup = self._inMemoryPowerups.get(interface, None) if inMemoryPowerup is not None: yield inMemoryPowerup + + try: + pups = self._dbPowerups[interface] + except KeyError: + pups = list(self._powerupsFor(interface)) + else: + pups = [(prio, p) for (prio, p) in pups if p._currentlyValidAsReferentFor(self.store)] + self._dbPowerups[interface] = pups + + for (prio, pup) in pups: + indirector = IPowerupIndirector(pup, None) + if indirector is not None: + yield indirector.indirect(interface) + else: + yield pup + + + def _powerupsFor(self, interface): + """ + Retrieve powerups from the database. + """ name = unicode(qual(interface), 'ascii') for cable in self.store.query( _PowerupConnector, @@ -352,11 +388,7 @@ def powerupsFor(self, interface): # this powerup was probably deleted during an upgrader. cable.deleteFromStore() else: - indirector = IPowerupIndirector(pup, None) - if indirector is not None: - yield indirector.indirect(interface) - else: - yield pup + yield (cable.priority, pup) def interfacesFor(self, powerup): """ @@ -495,6 +527,8 @@ class Item(Empowered, slotmachine._Strict): # A mapping from interfaces to in-memory powerups. _inMemoryPowerups = inmemory() + # A mapping from interfaces to in-database powerups. + _dbPowerups = inmemory() def _currentlyValidAsReferentFor(self, store): """ @@ -587,6 +621,7 @@ def __subinit__(self, **kw): """ self._axiom_service = None self._inMemoryPowerups = {} + self._dbPowerups = {} self.__dirty__ = {} to__store = kw.pop('__store', None) to__everInserted = kw.pop('__everInserted', False) diff --git a/axiom/store.py b/axiom/store.py index afeecabe..48c064ab 100644 --- a/axiom/store.py +++ b/axiom/store.py @@ -652,7 +652,13 @@ def itemsToDelete(attr): self.tableClass, itemsToDelete): it.deleteFromStore() - # actually run the DELETE for the items in this query. + # Slow-delete items in this query that are in the object cache. + for storeID in self.getColumn('storeID'): + if self.store.objectCache.has(storeID): + obj = self.store.objectCache.get(storeID) + obj.deleteFromStore() + + # Run the DELETE for the rest of the items in this query. self._runQuery('DELETE', "") class MultipleItemQuery(BaseQuery): @@ -1113,6 +1119,7 @@ def __init__(self, dbdir=None, filesdir=None, debug=False, parent=None, idInPare self.execTimes = [] self._inMemoryPowerups = {} + self._dbPowerups = {} self._attachedChildren = {} # database name => child store object diff --git a/axiom/test/test_powerup.py b/axiom/test/test_powerup.py index dc149060..67276102 100644 --- a/axiom/test/test_powerup.py +++ b/axiom/test/test_powerup.py @@ -1,3 +1,4 @@ +import gc from twisted.trial import unittest @@ -104,6 +105,20 @@ def __getPowerupInterfaces__(self, pifs): return 'not a list of pairs' + +class CountingItem(Item): + """ + An Item that increments a counter every time it is loaded from the + database. + """ + stuff = integer() + + count = 0 + + def activate(self): + CountingItem.count += 1 + + class PowerUpTest(unittest.TestCase): def testBasicPowerups(self): @@ -238,6 +253,95 @@ def testNoIndirectedIndirection(self): self.assertEqual(list(s.powerupsFor(IPowerupIndirector)), []) + def test_powerDownDeleted(self): + """ + If a powerup is deleted without powering down first, it should be + powered down automatically. + """ + s = Store() + mm = Summer(store=s) + s.powerUp(mm, ISumProducer) + mm.deleteFromStore() + self.assertEqual(list(s.powerupsFor(ISumProducer)), []) + + + +class CachingTests(unittest.TestCase): + """ + Tests for powerup caching behaviour. + """ + def test_powerupCaching(self): + """ + Powerups for an item should only be loaded from database once so long + as that item remains in memory. + """ + s = Store() + CountingItem.count = 0 + + ci = CountingItem(store=s) + s.powerUp(ci, ISumProducer) + self.assertEqual(CountingItem.count, 1) + del ci + gc.collect() + + pups = s.powerupsFor(ISumProducer) + self.assertEqual(CountingItem.count, 1) + + + def test_deleteCachedPowerup(self): + """ + A cached powerup that is deleted should be removed from the list of + installed powerups. + """ + s = Store() + ci = CountingItem(store=s) + s.powerUp(ci, ISumProducer) + + pups = list(s.powerupsFor(ISumProducer)) + self.assertEqual(pups, [ci]) + + ci.deleteFromStore() + pups2 = list(s.powerupsFor(ISumProducer)) + self.assertEqual(pups2, []) + + + def test_newCachedPowerup(self): + """ + Adding a new powerup when there are already cached powerups should add + the powerup to the cached list in the correct order. + """ + s = Store() + ci = CountingItem(store=s) + s.powerUp(ci, ISumProducer) + + pups = list(s.powerupsFor(ISumProducer)) + self.assertEqual(pups, [ci]) + + ci2 = CountingItem(store=s) + s.powerUp(ci2, ISumProducer, priority=50) + pups2 = list(s.powerupsFor(ISumProducer)) + self.assertEqual(pups2, [ci2, ci]) + + + def test_newCachedPowerupIndirection(self): + """ + Adding a new indirect powerup when there are already cached powerups + should add the powerup to the cached list. + """ + s = Store() + s.powerUp( + SubtractThree( + store=s, valueHaver=SumContributor(store=s, value=5)), + IValueHaver) + self.assertEqual(len(list(s.powerupsFor(IValueHaver))), 1) + + s.powerUp( + SubtractThree( + store=s, valueHaver=SumContributor(store=s, value=5)), + IValueHaver) + self.assertEqual(len(list(s.powerupsFor(IValueHaver))), 2) + + from twisted.application.service import IService, Service diff --git a/axiom/test/test_reference.py b/axiom/test/test_reference.py index 39f1322c..e1f18164 100644 --- a/axiom/test/test_reference.py +++ b/axiom/test/test_reference.py @@ -62,6 +62,22 @@ def testBadReferenceNone(self): (referent,) = list(store.query(SimpleReferent)) self.assertEqual(referent.ref, None) + + def test_badReferencedNoneCache(self): + """ + Test that accessing a broken reference to an Item that has been + deleted, but is still in the object cache, correctly nullifies the + attribute. + """ + store = Store() + def _test(): + referee = Referee(store=store, topSecret=0) + referent = SimpleReferent(store=store, ref=referee) + referee.deleteFromStore() + self.assertEqual(referent.ref, None) + store.transact(_test) + + def testBadReferenceNoneLoading(self): """ Test that accessing a broken reference on an Item that has not yet been diff --git a/axiom/test/test_xatop.py b/axiom/test/test_xatop.py index b5183453..2bfa5071 100644 --- a/axiom/test/test_xatop.py +++ b/axiom/test/test_xatop.py @@ -652,6 +652,11 @@ class StricterItem(item.Item): aRef = attributes.reference(allowNone=False) + +class ReferringItem(item.Item): + aRef = attributes.reference(whenDeleted=attributes.reference.NULLIFY) + + class AttributeTests(unittest.TestCase): def testGetAttribute(self): s = store.Store() @@ -957,6 +962,17 @@ def test_slowBatchDeleteBecauseDeletedFromStore(self): self.assertEqual(DeleteFromStoreTrackingItem.deletedTimes, 1) + def test_batchDeleteReference(self): + """ + Ensure that a reference to an Item deleted in a batch delete, but still + in the object cache, is nullified correctly. + """ + referee = AttributefulItem(store=self.store, withoutDefault=42) + referent = ReferringItem(store=self.store, aRef=referee) + self.store.query(AttributefulItem).deleteFromStore() + self.assertEqual(referent.aRef, None) + + # Item types we will use to change the underlying database schema (by creating # them). class ConcurrentItemA(item.Item): diff --git a/benchmark/batch-deletion b/benchmark/batch-deletion new file mode 100755 index 00000000..1253e64e --- /dev/null +++ b/benchmark/batch-deletion @@ -0,0 +1,44 @@ +#!/usr/bin/python + +# Benchmark of batch Item deletion. Accepts no parameters. Reports one +# statistic, the number of milliseconds it takes to batch delete an Item. + +import sys, time + +from axiom.store import Store +from axiom.attributes import integer + +import benchlib + + +def benchmark(): + SomeItem = benchlib.itemTypeWithSomeAttributes([integer]) + + store = Store() + outerCounter = range(10) + innerCounter = range(1000) + + def _create(): + benchlib.createSomeItems(store, SomeItem, {}, innerCounter) + + def _delete(): + store.query(SomeItem).deleteFromStore() + + timer = benchlib.StopWatch() + for i in outerCounter: + store.transact(_create) + timer.start() + store.transact(_delete) + timer.stop() + + return (timer.duration) * 1000 / (len(outerCounter) * len(innerCounter)) + + +def main(argv): + if len(argv) != 1: + raise SystemExit("Usage: %s" % (argv[0],)) + print benchmark() + + +if __name__ == '__main__': + main(sys.argv) diff --git a/benchmark/benchlib.py b/benchmark/benchlib.py index e36acc3b..1cbe032b 100644 --- a/benchmark/benchlib.py +++ b/benchmark/benchlib.py @@ -3,11 +3,28 @@ Helper functions useful to more than one benchmark script. """ +import time from itertools import count from axiom.item import Item from axiom.attributes import integer +class StopWatch(object): + """ + A very basic stopwatch timer. + """ + def __init__(self): + self.duration = 0.0 + + def start(self): + self.started = time.time() + + def stop(self): + self.finished = time.time() + self.duration += (self.finished - self.started) + + + typeNameCounter = count(0).next def itemTypeWithSomeAttributes(attributeTypes): @@ -22,6 +39,7 @@ class SomeItem(Item): return SomeItem + def createSomeItems(store, itemType, values, counter): """ Create some instances of a particular type in a store. diff --git a/benchmark/powerup-loading b/benchmark/powerup-loading new file mode 100755 index 00000000..93e9199d --- /dev/null +++ b/benchmark/powerup-loading @@ -0,0 +1,58 @@ +#!/usr/bin/python + +# Benchmark of Axiom powerup loading. Accepts no parameters. Reports one +# statistic, the number of milliseconds it takes to query for powerups after they +# have already been queried for once. + + +import sys, time + +from zope.interface import Interface + +from axiom.store import Store +from axiom.attributes import integer + +import benchlib + + +class IPowerup(Interface): + """ + Empty interface to use for powerups. + """ + + +def benchmark(): + SomeItem = benchlib.itemTypeWithSomeAttributes([integer]) + + store = Store() + items = [] + for i in range(10): + item = SomeItem(store=store) + store.powerUp(item, IPowerup) + items.append(item) + + + def _getPowerups(counter): + for i in counter: + list(store.powerupsFor(IPowerup)) + + + # cache the powerups first + store.transact(_getPowerups, range(1)) + + counter = range(1000) + before = time.time() + store.transact(_getPowerups, counter) + after = time.time() + + return (after - before) * 1000 / (len(counter)) + + +def main(argv): + if len(argv) != 1: + raise SystemExit("Usage: %s") + print benchmark() + + +if __name__ == '__main__': + main(sys.argv)