Use decorators to replace a few more manual checks of self.keep_history.

relstorage/adapters/_util.py

@@ -15,6 +15,7 @@
 
 from functools import partial
 from functools import update_wrapper
+from functools import wraps
 
 from relstorage._compat import intern
 
@@ -83,3 +84,54 @@ def prop(inst):
     return Lazy(prop, base_name + '_query')
 
 formatted_query_property = partial(query_property, formatted=True)
+
+
+def noop_when_history_free(meth):
+    """
+    Decorator for *meth* that causes it to do nothing when
+    ``self.keep_history`` is False.
+
+    *meth* must have no return value (returns None) when it is
+    history free. When history is preserved it can return anything.
+
+    This requires a bit more memory to use the instance dict, but at
+    runtime it has minimal time overhead (after the first call).
+    """
+
+    # Python 3.4 (via timeit)
+    # calling a trivial method ('def t(self, arg): return arg') takes 118ns
+    # calling a method that does 'if not self.keep_history: return; return arg'
+    #   takes 142 ns
+    # calling a functools.partial bound to self wrapped around t
+    #   takes 298ns
+    # calling a generic python function
+    #     def wrap(self, *args, **kwargs):
+    #       if not self.keep_history: return
+    #       return self.t(*args, **kwargs)
+    #   takes 429ns
+    # So a partial function set into the __dict__ is the fastest way to
+    # do this.
+
+    meth_name = meth.__name__
+
+    @wraps(meth)
+    def no_op(*_args, **_kwargs):
+        return
+
+    @wraps(meth)
+    def swizzler(self, *args, **kwargs):
+        if not self.keep_history:
+            setattr(self, meth_name, no_op)
+        else:
+            # NOTE: This creates a reference cycle
+            bound = partial(meth, self)
+            update_wrapper(bound, meth)
+            setattr(self, meth_name, bound)
+
+        return getattr(self, meth_name)(*args, **kwargs)
+
+    if not hasattr(swizzler, '__wrapped__'):
+        # Py2: this was added in 3.2
+        swizzler.__wrapped__ = meth
+
+    return swizzler

relstorage/adapters/mover.py

@@ -18,20 +18,13 @@
 from relstorage.adapters.batch import RowBatcher
 from relstorage.adapters.interfaces import IObjectMover
 from relstorage.adapters._util import query_property as _query_property
+from relstorage.adapters._util import noop_when_history_free
 from relstorage.iter import fetchmany
 from zope.interface import implementer
 from hashlib import md5
 
 from relstorage._compat import db_binary_to_bytes
 
-def compute_md5sum(data):
-    if data is not None:
-        return md5(data).hexdigest()
-    else:
-        # George Bailey object
-        return None
-
-
 metricmethod_sampled = Metric(method=True, rate=0.1)
 
 @implementer(IObjectMover)
@@ -48,10 +41,11 @@ def __init__(self, database_type, options, runner=None,
         self.version_detector = version_detector
         self.make_batcher = batcher_factory
 
-        if self.keep_history:
-            self._compute_md5sum = compute_md5sum
-        else:
-            self._compute_md5sum = lambda arg: None
+    @noop_when_history_free
+    def _compute_md5sum(self, data):
+        if data is None:
+            return None
+        return md5(data).hexdigest()
 
     _load_current_queries = (
         """
@@ -415,17 +409,14 @@ def move_from_temp(self, cursor, tid, txn_has_blobs):
             ORDER BY zoid)
         """
 
+    @noop_when_history_free
     @metricmethod_sampled
-    def update_current(self, cursor, tid): # pylint:disable=method-hidden
-        """Update the current object pointers.
+    def update_current(self, cursor, tid):
+        """
+        Update the current object pointers.
 
         tid is the integer tid of the transaction being committed.
         """
-        if not self.keep_history:
-            # nothing needs to be updated
-            # Can elide this check in the future.
-            self.update_current = lambda cursor, tid: None
-            return
 
         stmt = self._update_current_insert_query
         cursor.execute(stmt, (tid,))

relstorage/adapters/mysql/mover.py

@@ -22,6 +22,7 @@
 from ..mover import AbstractObjectMover
 from ..mover import metricmethod_sampled
 from .._util import query_property
+from .._util import noop_when_history_free
 
 
 def to_prepared_queries(name, queries, extension=''):
@@ -100,18 +101,14 @@ def _move_from_temp_object_state(self, cursor, tid):
         """
         cursor.execute(stmt, (tid,))
 
+    @noop_when_history_free
     @metricmethod_sampled
     def update_current(self, cursor, tid):  # pylint:disable=method-hidden
-        """Update the current object pointers.
+        """
+        Update the current object pointers.
 
         tid is the integer tid of the transaction being committed.
         """
-        if not self.keep_history:
-            # nothing needs to be updated
-            # Can elide this check in the future.
-            self.update_current = lambda cursor, tid: None
-            return
-
         cursor.execute("""
         REPLACE INTO current_object (zoid, tid)
         SELECT zoid, tid FROM object_state

relstorage/adapters/tests/test_util.py

@@ -0,0 +1,108 @@
+##############################################################################
+#
+# Copyright (c) 2017 Zope Foundation and Contributors.
+# All Rights Reserved.
+#
+# This software is subject to the provisions of the Zope Public License,
+# Version 2.1 (ZPL).  A copy of the ZPL should accompany this distribution.
+# THIS SOFTWARE IS PROVIDED "AS IS" AND ANY AND ALL EXPRESS OR IMPLIED
+# WARRANTIES ARE DISCLAIMED, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+# WARRANTIES OF TITLE, MERCHANTABILITY, AGAINST INFRINGEMENT, AND FITNESS
+# FOR A PARTICULAR PURPOSE.
+#
+##############################################################################
+
+import unittest
+
+from relstorage.adapters._util import noop_when_history_free
+from ..mover import metricmethod_sampled
+
+class TestNoOp(unittest.TestCase):
+
+
+    def test_history_free(self):
+
+        class Thing(object):
+
+            keep_history = False
+
+            @noop_when_history_free
+            @metricmethod_sampled
+            def thing(self, arg1, arg2=False):
+                "Docs"
+                assert arg1
+                assert arg2
+
+
+        thing = Thing()
+        # Before calling it has a wrapper
+        thing_thing = thing.thing
+        self.assertTrue(hasattr(thing_thing, '__wrapped__'))
+        self.assertEqual(Thing.thing.__doc__, "Docs")
+        self.assertEqual(Thing.thing.__doc__, thing_thing.__doc__)
+        self.assertEqual(Thing.thing.__name__, 'thing')
+        self.assertEqual(Thing.thing.__name__, thing_thing.__name__)
+
+        # call with false args to be sure doesn't run
+        thing.thing(0) # one argument
+        thing.thing(0, arg2=0) # kwarg
+
+        # It still has a wrapper, but it's not the original
+        self.assertTrue(hasattr(thing.thing, '__wrapped__'))
+        self.assertIsNot(thing.thing, thing_thing)
+        self.assertEqual(Thing.thing.__doc__, thing.thing.__doc__)
+        self.assertEqual(Thing.thing.__name__, 'thing')
+        self.assertEqual(Thing.thing.__name__, thing.thing.__name__)
+
+    def test_keep_history(self):
+
+        class Thing(object):
+
+            keep_history = True
+            ran = False
+
+            @noop_when_history_free
+            def thing(self, arg1, arg2=False):
+                "Docs"
+                assert arg1
+                assert arg2
+                self.ran = True
+                return arg1
+
+
+        thing = Thing()
+        # Before calling it has a wrapper
+        thing_thing = thing.thing
+        self.assertTrue(hasattr(thing_thing, '__wrapped__'))
+        self.assertEqual(Thing.thing.__doc__, "Docs")
+        self.assertEqual(Thing.thing.__doc__, thing_thing.__doc__)
+        self.assertEqual(Thing.thing.__name__, 'thing')
+        self.assertEqual(Thing.thing.__name__, thing_thing.__name__)
+
+        # call with false args to be sure does run
+        self.assertRaises(AssertionError, thing.thing, 0) # one argument
+        thing.thing(1, arg2=1)
+        self.assertRaises(AssertionError, thing.thing, 1, arg2=0)
+
+        # It has access to self
+        self.assertTrue(thing.ran)
+
+        # It returns a value...
+        self.assertIs(self, thing.thing(self, 1))
+        # ...even the first time its called
+        self.assertIs(self, Thing().thing(self, 1))
+
+        # It still has a wrapper, but it's not the original
+        self.assertTrue(hasattr(thing.thing, '__wrapped__'))
+        self.assertIsNot(thing.thing, thing_thing)
+        self.assertEqual(Thing.thing.__doc__, thing.thing.__doc__)
+        self.assertEqual(Thing.thing.__name__, 'thing')
+        self.assertEqual(Thing.thing.__name__, thing.thing.__name__)
+
+def test_suite():
+    suite = unittest.TestSuite()
+    suite.addTest(unittest.makeSuite(TestNoOp))
+    return suite
+
+if __name__ == '__main__':
+    unittest.main(defaultTest='test_suite')

relstorage/adapters/txncontrol.py

@@ -22,7 +22,8 @@
 from zope.interface import implementer
 
 from .interfaces import ITransactionControl
-
+from ._util import query_property
+from ._util import noop_when_history_free
 
 @six.add_metaclass(abc.ABCMeta)
 class AbstractTransactionControl(object):
@@ -70,23 +71,19 @@ class GenericTransactionControl(AbstractTransactionControl):
     and history-preserving storages that share a common syntax.
     """
 
-    # XXX: Use the query_property for this
-    _GET_TID_HP = "SELECT MAX(tid) FROM transaction"
-    _GET_TID_HF = "SELECT MAX(tid) FROM object_state"
+
+    _get_tid_queries = (
+        "SELECT MAX(tid) FROM transaction",
+        "SELECT MAX(tid) FROM object_state"
+    )
+    _get_tid_query = query_property('_get_tid')
 
     def __init__(self, keep_history, Binary): # noqa
         self.keep_history = keep_history
         self.Binary = Binary
 
-        if keep_history:
-            self.add_transaction = self._add_transaction_preserve
-            self._get_tid_stmt = self._GET_TID_HP
-        else:
-            self.add_transaction = self._add_transaction_free
-            self._get_tid_stmt = self._GET_TID_HF
-
     def get_tid(self, cursor):
-        cursor.execute(self._get_tid_stmt)
+        cursor.execute(self._get_tid_query)
         row = cursor.fetchone()
         if not row:
             # nothing has been stored yet
@@ -95,14 +92,9 @@ def get_tid(self, cursor):
         tid = row[0]
         return tid if tid is not None else 0
 
-
-    def _add_transaction_free(self, cursor, tid, username, description, extension,
-                              packed=False):
-        # pylint:disable=unused-argument
-        return
-
-    def _add_transaction_preserve(self, cursor, tid, username, description, extension,
-                                  packed=False):
+    @noop_when_history_free
+    def add_transaction(self, cursor, tid, username, description, extension,
+                        packed=False):
         stmt = """
         INSERT INTO transaction
             (tid, packed, username, description, extension)
@@ -112,5 +104,3 @@ def _add_transaction_preserve(self, cursor, tid, username, description, extensio
         cursor.execute(stmt, (
             tid, packed, binary(username),
             binary(description), binary(extension)))
-
-    add_transaction = lambda *args: None # dynamically replaced

relstorage/tests/alltests.py

@@ -30,6 +30,7 @@ def make_suite():
         'relstorage.adapters.tests.test_batch',
         'relstorage.adapters.tests.test_connmanager',
         'relstorage.adapters.tests.test_replica',
+        'relstorage.adapters.tests.test_util',
         'relstorage.cache.tests.test_cache',
         'relstorage.cache.tests.test_cache_stats',
         'relstorage.tests.test_autotemp',