divmod-dev team mailing list archive

[Tristan Seligmann <mithrandi@xxxxxxxxxxxxx>]

Tristan Seligmann has proposed merging lp:~divmod-dev/divmod.org/829879-pypy-finalization-semantics into lp:divmod.org with lp:~divmod-dev/divmod.org/optional-pycrypto as a prerequisite.

Requested reviews:
  Divmod-dev (divmod-dev)
Related bugs:
  Bug #829879 in Divmod Axiom: "Axiom finalization cache does not work on PyPy"
  https://bugs.launchpad.net/divmod-axiom/+bug/829879

For more details, see:
https://code.launchpad.net/~divmod-dev/divmod.org/829879-pypy-finalization-semantics/+merge/72279
-- 
https://code.launchpad.net/~divmod-dev/divmod.org/829879-pypy-finalization-semantics/+merge/72279
Your team Divmod-dev is requested to review the proposed merge of lp:~divmod-dev/divmod.org/829879-pypy-finalization-semantics into lp:divmod.org.

=== modified file 'Axiom/axiom/_fincache.py'
--- Axiom/axiom/_fincache.py	2006-04-01 19:46:29 +0000
+++ Axiom/axiom/_fincache.py	2011-08-20 02:28:24 +0000
@@ -1,4 +1,3 @@
-
 from weakref import ref
 from traceback import print_exc
 
@@ -6,11 +5,12 @@
 
 from axiom import iaxiom
 
-class CacheFault(RuntimeError):
-    """
-    A serious problem has occurred within the cache.  This error is internal
-    and should never really be trapped.
-    """
+class CacheFault(KeyError):
+    """
+    An item has fallen out of cache, but the weakref callback has not yet run.
+    """
+
+
 
 def logErrorNoMatterWhat():
     try:
@@ -27,6 +27,8 @@
             # to.  Don't bother.
             return
 
+
+
 def createCacheRemoveCallback(w, k, f):
     def remove(self):
         # Weakref callbacks cannot raise exceptions or DOOM ensues
@@ -37,12 +39,16 @@
         try:
             self = w()
             if self is not None:
-                del self.data[k]
+                try:
+                    del self.data[k]
+                except KeyError:
+                    # Already gone
+                    pass
         except:
             logErrorNoMatterWhat()
     return remove
 
-PROFILING = False
+
 
 class FinalizingCache:
     """Possibly useful for infrastructure?  This would be a nice addition (or
@@ -50,9 +56,7 @@
     """
     def __init__(self):
         self.data = {}
-        if not PROFILING:
-            # see docstring for 'has'
-            self.has = self.data.has_key
+
 
     def cache(self, key, value):
         fin = value.__finalizer__()
@@ -61,28 +65,28 @@
                 ref(self), key, fin))
         return value
 
+
     def uncache(self, key, value):
-        assert self.get(key) is value
-        del self.data[key]
-
-    def has(self, key):
-        """Does the cache have this key?
-
-        (This implementation is only used if the system is being profiled, due
-        to bugs in Python's old profiler and its interaction with weakrefs.
-        Set the module attribute PROFILING to True at startup for this.)
-        """
-        if key in self.data:
-            o = self.data[key]()
-            if o is None:
-                del self.data[key]
-                return False
-            return True
-        return False
+        """
+        Remove a key from the cache.
+
+        As a sanity check, if the specified key is present in the cache, it
+        must have the given value.
+
+        @param key: The key to remove.
+        @param value: The expected value for the key.
+        """
+        try:
+            assert self.get(key) is value
+            del self.data[key]
+        except KeyError:
+            pass
+
 
     def get(self, key):
         o = self.data[key]()
         if o is None:
+            del self.data[key]
             raise CacheFault(
                 "FinalizingCache has %r but its value is no more." % (key,))
         log.msg(interface=iaxiom.IStatEvent, stat_cache_hits=1, key=key)

=== modified file 'Axiom/axiom/store.py'
--- Axiom/axiom/store.py	2009-07-07 20:35:43 +0000
+++ Axiom/axiom/store.py	2011-08-20 02:28:24 +0000
@@ -1741,10 +1741,10 @@
             self.executeSQL(sql, insertArgs)
 
     def _loadedItem(self, itemClass, storeID, attrs):
-        if self.objectCache.has(storeID):
+        try:
             result = self.objectCache.get(storeID)
             # XXX do checks on consistency between attrs and DB object, maybe?
-        else:
+        except KeyError:
             result = itemClass.existingInStore(self, storeID, attrs)
             if not result.__legacy__:
                 self.objectCache.cache(storeID, result)
@@ -2199,8 +2199,10 @@
                     type(storeID).__name__,))
         if storeID == STORE_SELF_ID:
             return self
-        if self.objectCache.has(storeID):
+        try:
             return self.objectCache.get(storeID)
+        except KeyError:
+            pass
         log.msg(interface=iaxiom.IStatEvent, stat_cache_misses=1, key=storeID)
         results = self.querySchemaSQL(_schema.TYPEOF_QUERY, [storeID])
         assert (len(results) in [1, 0]),\

=== modified file 'Axiom/axiom/test/test_reference.py'
--- Axiom/axiom/test/test_reference.py	2009-07-06 12:35:46 +0000
+++ Axiom/axiom/test/test_reference.py	2011-08-20 02:28:24 +0000
@@ -186,7 +186,7 @@
         store = Store()
         t = nonUpgradedItem(store=store)
         self.assertEquals(t.__legacy__, True)
-        self.assertFalse(store.objectCache.has(t.storeID))
+        self.assertRaises(KeyError, store.objectCache.get, t.storeID)
         t2 = store.getItemByID(t.storeID)
         self.assertNotIdentical(t, t2)
         self.assertTrue(isinstance(t2, UpgradedItem))