Monkey-patch FieldProperty so we can avoid double-validation.

Fixes #67

Add tests that this doesn't break anything.

In our simple tests, this makes us at least 10% faster.

2.7:

| Benchmark                    | 27_list_fieldproperty3 | 27_list_fieldproperty_patched3 |
|------------------------------|------------------------|--------------------------------|
| __main__: fromExternalObject | 1.43 ms                | 1.27 ms: 1.12x faster (-11%)   |

Not significant (1): __main__: toExternalObject

3.6 (taken during a backup, results for toExternalObject had a *huge*
stddev of 40%):

| Benchmark                    | 36_list_fieldproperty | 36_list_fieldproperty_patched2 |
|------------------------------|-----------------------|--------------------------------|
| __main__: toExternalObject   | 428 us                | 577 us: 1.35x slower (+35%)    |
| __main__: fromExternalObject | 1.43 ms               | 1.19 ms: 1.20x faster (-17%)   |

CHANGES.rst

@@ -6,6 +6,12 @@
 1.0.0a3 (unreleased)
 ====================
 
+- Updating objects that use ``createFieldProperties`` or otherwise
+  have ``FieldProperty`` objects in their type is at least 10% faster
+  thanks to avoiding double-validation due to a small monkey-patch on
+  ``FieldProperty``. See `issue 67
+  <https://github.com/NextThought/nti.externalization/issues/67>`_.
+
 - Proxies around objects that implement ``toExternalObject`` are
   allowed again; the proxied object's ``toExternalObject`` will be called.
 

src/nti/externalization/internalization/_fields.pxd

@@ -18,11 +18,19 @@ cdef ValidationError
 cdef WrongContainedType
 cdef WrongType
 
+cdef FieldProperty
+cdef NO_VALUE
+cdef FieldUpdatedEvent
+
+cdef notify
+
 # optimizations
 cdef IField_providedBy
 
 
 cdef noop()
+cpdef _FieldProperty__set__valid(self, inst, value)
+cdef _FieldProperty_orig_set
 
 
 @cython.final

src/nti/externalization/internalization/fields.py

@@ -10,6 +10,7 @@
 from __future__ import division
 from __future__ import print_function
 
+# pylint:disable=protected-access
 
 # stdlib imports
 import sys
@@ -26,6 +27,12 @@
 from zope.schema.interfaces import WrongContainedType
 from zope.schema.interfaces import WrongType
 
+from zope.schema.fieldproperty import FieldProperty
+from zope.schema.fieldproperty import NO_VALUE
+from zope.schema.fieldproperty import FieldUpdatedEvent
+
+from zope.event import notify
+
 IField_providedBy = IField.providedBy
 
 __all__ = [
@@ -78,11 +85,51 @@ def __init__(self, ext_self, field, value):
         self.ext_self = ext_self
         # Don't denormalize field.set; there's a tiny
         # chance we won't actually be called.
+        # The field must already be bound to ext_self, and
+        # the value must already be valid.
         self.field = field
         self.value = value
 
     def __call__(self):
-        self.field.set(self.ext_self, self.value)
+        # We monkey-patch FieldProperty so we can avoid double
+        # validation, which can be quite expensive in benchmarks.
+        # (See below.)
+
+        # The object we're updating is either newly created or
+        # otherwise local to this thread, so there shouldn't be any
+        # race conditions here. We also generally don't expect to be used
+        # with objects that have limited __slots__ and no __dict__
+        self.ext_self._v_bound_field_already_valid = self.field
+        try:
+            self.field.set(self.ext_self, self.value)
+        finally:
+            del self.ext_self._v_bound_field_already_valid
+
+
+_FieldProperty_orig_set = FieldProperty.__set__
+
+def _FieldProperty__set__valid(self, inst, value):
+    valid_field = getattr(inst, '_v_bound_field_already_valid', None)
+    if valid_field is not None:
+        # Skip the validation, but do everything else just like
+        # FieldProperty does.
+        oldvalue = self.queryValue(inst, NO_VALUE)
+        inst.__dict__[self._FieldProperty__name] = value
+        notify(FieldUpdatedEvent(inst, valid_field, oldvalue, value))
+    else:
+        _FieldProperty_orig_set(self, inst, value)
+
+_FieldProperty__set__valid.orig_func = _FieldProperty_orig_set
+
+# Detect the case that we're in Cython compiled code, where
+# we've already replaced the __set__ function with our own.
+if FieldProperty.__set__.__name__ == _FieldProperty__set__valid.__name__: # pragma: no cover
+    _FieldProperty_orig_set = FieldProperty.__set__.orig_func
+    _FieldProperty__set__valid.org_func = _FieldProperty_orig_set
+
+FieldProperty.__set__ = _FieldProperty__set__valid
+
+
 
 class CannotConvertSequenceError(TypeError):
     """

src/nti/externalization/internalization/tests/test_fields.py

@@ -0,0 +1,95 @@
+# -*- coding: utf-8 -*-
+from __future__ import absolute_import
+from __future__ import division
+from __future__ import print_function
+
+import unittest
+
+from zope import interface
+from zope.component import eventtesting
+
+
+from zope.schema.interfaces import IBeforeObjectAssignedEvent
+from zope.schema.interfaces import IFieldUpdatedEvent
+from zope.schema.interfaces import SchemaNotProvided
+from zope.schema import Object
+from zope.schema.fieldproperty import createFieldProperties
+
+from zope.testing.cleanup import CleanUp
+
+from hamcrest import assert_that
+from hamcrest import has_length
+from hamcrest import has_property as has_attr
+from hamcrest import is_
+from hamcrest import same_instance
+
+from nti.externalization.internalization.fields import validate_named_field_value
+
+# pylint:disable=inherit-non-class
+
+class IThing(interface.Interface):
+    pass
+
+class IBar(interface.Interface):
+
+    thing = Object(IThing)
+
+@interface.implementer(IThing)
+class Thing(object):
+    pass
+
+@interface.implementer(IBar)
+class Bar(object):
+    createFieldProperties(IBar)
+
+class TestValidateFieldValueEvents(CleanUp,
+                                   unittest.TestCase):
+
+    def setUp(self):
+        eventtesting.setUp()
+
+
+    def test_before_object_assigned_event_fired_valid_value(self):
+
+        thing = Thing()
+        root = Bar()
+
+        validate_named_field_value(root, IBar, 'thing', thing)()
+
+        events = eventtesting.getEvents(IBeforeObjectAssignedEvent)
+        assert_that(events, has_length(1))
+        assert_that(events[0], has_attr('object', is_(same_instance(thing))))
+
+        events = eventtesting.getEvents(IFieldUpdatedEvent)
+        assert_that(events, has_length(1))
+
+
+    def test_before_object_assigned_event_fired_invalid_value_fixed(self):
+        thing = Thing()
+        root = Bar()
+
+        class NotThing(object):
+            def __conform__(self, iface):
+                return thing if iface is IThing else None
+
+
+        validate_named_field_value(root, IBar, 'thing', NotThing())()
+
+        events = eventtesting.getEvents(IBeforeObjectAssignedEvent)
+        assert_that(events, has_length(1))
+        assert_that(events[0], has_attr('object', is_(same_instance(thing))))
+
+        events = eventtesting.getEvents(IFieldUpdatedEvent)
+        assert_that(events, has_length(1))
+
+
+    def test_before_object_assigned_event_not_fired_invalid_value(self):
+
+        with self.assertRaises(SchemaNotProvided):
+            validate_named_field_value(Bar(), IBar, 'thing', object())
+
+        events = eventtesting.getEvents(IBeforeObjectAssignedEvent)
+        assert_that(events, has_length(0))
+
+        events = eventtesting.getEvents(IFieldUpdatedEvent)
+        assert_that(events, has_length(0))