From d73800c8f19864daf97951cd9a8b7f28c5aabfeb Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Tue, 16 Mar 2021 14:56:15 -0500 Subject: [PATCH 1/2] Make PersistentAdapterRegistry use PersistentMapping/PersistentList with zope.interface 5.3+ Fixes #51 --- CHANGES.rst | 18 ++++++ src/zope/component/persistentregistry.py | 60 +++++++++++++++++++ .../tests/test_persistentregistry.py | 44 ++++++++++++-- 3 files changed, 116 insertions(+), 6 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index b83b913..dd70049 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -26,6 +26,24 @@ before. See `issue 9 `_. +- Make ``PersistentAdapterRegistry`` use persistent objects + (``PersistentMapping`` and ``PersistentList``) for its internal data + structures instead of plain dicts and lists. This helps make it + scalable to larger registry sizes. + + To take advantage of this, you need zope.interface 5.3 or later + (earlier versions continue to work, but do not allow this + optimization). + + New registries (and their primary user, ``PersistentComponents`` and + zope.site's ``LocalSiteManager``) take full advantage of this + automatically. For existing persistent registries to take advantage + of this, you must call their ``rebuild()`` method and commit the + transaction. + + See `issue 51 `_. + + 4.6.2 (2020-07-03) ================== diff --git a/src/zope/component/persistentregistry.py b/src/zope/component/persistentregistry.py index 9c3ae5c..f01111c 100644 --- a/src/zope/component/persistentregistry.py +++ b/src/zope/component/persistentregistry.py @@ -22,9 +22,69 @@ class PersistentAdapterRegistry(VerifyingAdapterRegistry, Persistent): """ An adapter registry that is also a persistent object. + + .. versionchanged:: 4.7.0 + Internal data structures are now composed of + :class:`persistent.mapping.PersistentMapping` and + :class:`persistent.list.PersistentList`. This helps scale to + larger registries. + + Previously they were :class:`dict`, :class:`list` and + :class:`tuple`, meaning that as soon as this object was + unpickled, the entire registry tree had to be unpickled, and + when a change was made (an object registered or unregisterd), + the entire registry had to be pickled. Now, only the parts + that are in use are loaded, and only the parts that are + modified are stored. + + The above applies without reservation to newly created + instances. For existing persistent instances, however, the + data will continue to be in dicts and tuples, with some few + exceptions for newly added or changed data. + + To fix this, call :meth:`rebuild` and commit the transaction. + This will rewrite the internal data structures to use the new + types. """ + + # The persistent types we use, replacing the basic types inherited + # from ``BaseAdapterRegistry``. + _sequenceType = PersistentList + _leafSequenceType = PersistentList + _mappingType = PersistentMapping + _providedType = PersistentMapping + + # The methods needed to manipulate the leaves of the subscriber + # tree. When we're manipulating unmigrated data, it's safe to + # migrate it, but not otherwise (we don't want to write in an + # otherwise read-only transaction). + def _addValueToLeaf(self, existing_leaf_sequence, new_item): + if not existing_leaf_sequence: + existing_leaf_sequence = self._leafSequenceType() + elif isinstance(existing_leaf_sequence, tuple): + # Converting from old state. + existing_leaf_sequence = self._leafSequenceType(existing_leaf_sequence) + existing_leaf_sequence.append(new_item) + return existing_leaf_sequence + + def _removeValueFromLeaf(self, existing_leaf_sequence, to_remove): + without_removed = VerifyingAdapterRegistry._removeValueFromLeaf( + self, + existing_leaf_sequence, + to_remove) + existing_leaf_sequence[:] = without_removed + return existing_leaf_sequence + def changed(self, originally_changed): if originally_changed is self: + # XXX: This is almost certainly redundant, even if we + # have old data consisting of plain dict/list/tuple. That's + # because the super class will now increment the ``_generation`` + # attribute to keep caches in sync. For this same reason, + # it's not worth trying to "optimize" for the case that we're a + # new or rebuilt object containing only Persistent sub-objects: + # the changed() mechanism will still result in mutating this + # object via ``_generation``. self._p_changed = True super(PersistentAdapterRegistry, self).changed(originally_changed) diff --git a/src/zope/component/tests/test_persistentregistry.py b/src/zope/component/tests/test_persistentregistry.py index 9002e30..606ce10 100644 --- a/src/zope/component/tests/test_persistentregistry.py +++ b/src/zope/component/tests/test_persistentregistry.py @@ -15,6 +15,7 @@ """ import unittest +from zope.interface.tests.test_adapter import CustomTypesBaseAdapterRegistryTests def skipIfNoPersistent(testfunc): try: @@ -101,12 +102,13 @@ def test___getstate___simple(self): bases = (globalSiteManager.adapters, globalSiteManager.utilities) registry, jar, OID = self._makeOneWithJar(bases=bases) state = registry.__getstate__() - self.assertEqual(state['__bases__'], bases) - self.assertEqual(state['_generation'], 1) - self.assertEqual(state['_provided'], {}) - self.assertEqual(state['_adapters'], []) - self.assertEqual(state['_subscribers'], []) - self.assertFalse('ro' in state) + self.assertEqual(state.pop('__bases__'), bases) + self.assertEqual(state.pop('_generation'), 1) + self.assertEqual(state.pop('_provided'), {}) + self.assertEqual(state.pop('_adapters'), []) + self.assertEqual(state.pop('_subscribers'), []) + self.assertNotIn('ro', state) + self.assertEqual(state, {}) def test___getstate___skips_delegated_names(self): registry, jar, OID = self._makeOneWithJar() @@ -131,6 +133,14 @@ def test___setstate___rebuilds__ro(self): self.assertEqual(registry.__bases__, bases) self.assertEqual(registry.ro, [registry] + list(bases)) + def test__addValueToLeaf_existing_is_tuple_converts(self): + from persistent.list import PersistentList + registry = self._makeOne() + result = registry._addValueToLeaf(('a',), 'b') + self.assertIsInstance(result, PersistentList) + self.assertEqual(result, ['a', 'b']) + + @skipIfNoPersistent class PersistentComponentsTests(unittest.TestCase): @@ -161,3 +171,25 @@ def test_ctor_initializes_registries_and_registrations(self): def _makeOctets(s): return bytes(s) if bytes is str else bytes(s, 'ascii') + + +@skipIfNoPersistent +class PersistentAdapterRegistryCustomTypesTest(CustomTypesBaseAdapterRegistryTests): + + def _getMappingType(self): + from persistent.mapping import PersistentMapping + return PersistentMapping + + def _getProvidedType(self): + return self._getMappingType() + + def _getMutableListType(self): + from persistent.list import PersistentList + return PersistentList + + def _getLeafSequenceType(self): + return self._getMutableListType() + + def _getBaseAdapterRegistry(self): + from zope.component.persistentregistry import PersistentAdapterRegistry + return PersistentAdapterRegistry From f29353b0b89ac90e7d6242fa76b75e713ddf8c78 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Thu, 18 Mar 2021 09:15:03 -0500 Subject: [PATCH 2/2] Bump zope.interface dependency. --- CHANGES.rst | 6 ++---- setup.py | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index dd70049..3c49567 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -31,11 +31,9 @@ structures instead of plain dicts and lists. This helps make it scalable to larger registry sizes. - To take advantage of this, you need zope.interface 5.3 or later - (earlier versions continue to work, but do not allow this - optimization). + This requires zope.interface 5.3.0a1 or later. - New registries (and their primary user, ``PersistentComponents`` and + New registries (and their primary users, ``PersistentComponents`` and zope.site's ``LocalSiteManager``) take full advantage of this automatically. For existing persistent registries to take advantage of this, you must call their ``rebuild()`` method and commit the diff --git a/setup.py b/setup.py index 50dbeb5..117d110 100644 --- a/setup.py +++ b/setup.py @@ -109,7 +109,7 @@ def read(*rnames): 'setuptools', 'zope.event', 'zope.hookable >= 4.2.0', - 'zope.interface >= 4.1.0', + 'zope.interface >= 5.3.0a1', ], include_package_data=True, zip_safe=False,