From 54752533b2ed1c898ffe5ec2e795c6910ee46a39 Mon Sep 17 00:00:00 2001 From: Tal Einat Date: Mon, 10 Sep 2018 16:11:04 +0300 Subject: [PATCH] bpo-30977: rework code changes according to post-merge code review (GH-9106) also mention the change and its consequences in What's New --- Doc/whatsnew/3.8.rst | 7 ++ Lib/test/test_uuid.py | 176 ++++++++++++++++++++++++++++++------------ Lib/uuid.py | 27 +++---- 3 files changed, 144 insertions(+), 66 deletions(-) diff --git a/Doc/whatsnew/3.8.rst b/Doc/whatsnew/3.8.rst index 2de7a50a471..a9b689d5372 100644 --- a/Doc/whatsnew/3.8.rst +++ b/Doc/whatsnew/3.8.rst @@ -131,6 +131,10 @@ Optimizations objects (e.g. tuple, list, dict) size is reduced 4 or 8 bytes. (Contributed by Inada Naoki in :issue:`33597`) +* :class:`uuid.UUID` now uses ``__slots__`` to reduce its memory footprint. + Note that this means that instances can no longer be weak-referenced and + that arbitrary attributes can no longer be added to them. + Build and C API Changes ======================= @@ -275,6 +279,9 @@ Changes in the Python API * The function :func:`math.factorial` no longer accepts arguments that are not int-like. (Contributed by Pablo Galindo in :issue:`33083`.) +* :class:`uuid.UUID` now uses ``__slots__``, therefore instances can no longer + be weak-referenced and attributes can no longer be added. + CPython bytecode changes ------------------------ diff --git a/Lib/test/test_uuid.py b/Lib/test/test_uuid.py index 9ec59d51957..c1b35c4a7f6 100644 --- a/Lib/test/test_uuid.py +++ b/Lib/test/test_uuid.py @@ -2,6 +2,8 @@ import unittest.mock from test import support import builtins import contextlib +import copy +from functools import partial import io import os import pickle @@ -313,63 +315,139 @@ class BaseTestUUID: node2 = self.uuid.getnode() self.assertEqual(node1, node2, '%012x != %012x' % (node1, node2)) - def _setup_for_pickle(self): - orig_uuid = sys.modules.get('uuid') - sys.modules['uuid'] = self.uuid - - def restore_uuid_module(): - if orig_uuid is not None: - sys.modules['uuid'] = orig_uuid - else: - del sys.modules['uuid'] - self.addCleanup(restore_uuid_module) - def test_pickle_roundtrip(self): - self._setup_for_pickle() + def check(actual, expected): + self.assertEqual(actual, expected) + self.assertEqual(actual.is_safe, expected.is_safe) - u = self.uuid.UUID('12345678123456781234567812345678') - self.assertEqual(u, pickle.loads(pickle.dumps(u))) + with support.swap_item(sys.modules, 'uuid', self.uuid): + for is_safe in self.uuid.SafeUUID: + u = self.uuid.UUID('d82579ce6642a0de7ddf490a7aec7aa5', + is_safe=is_safe) + check(copy.copy(u), u) + check(copy.deepcopy(u), u) + for proto in range(pickle.HIGHEST_PROTOCOL + 1): + with self.subTest(protocol=proto): + check(pickle.loads(pickle.dumps(u, proto)), u) def test_unpickle_previous_python_versions(self): - self._setup_for_pickle() + def check(actual, expected): + self.assertEqual(actual, expected) + self.assertEqual(actual.is_safe, expected.is_safe) - u = self.uuid.UUID('12345678123456781234567812345678') - - # Python 2.7 protocol 0-2 pickles of u - py27_pickles = [ - b'ccopy_reg\n_reconstructor\np0\n(cuuid\nUUID\np1\nc__builtin__\nob' - b'ject\np2\nNtp3\nRp4\n(dp5\nS\'int\'\np6\nL24197857161011715162171' - b'839636988778104L\nsb.', - b'ccopy_reg\n_reconstructor\nq\x00(cuuid\nUUID\nq\x01c__builtin__\n' - b'object\nq\x02Ntq\x03Rq\x04}q\x05U\x03intq\x06L2419785716101171516' - b'2171839636988778104L\nsb.', - b'\x80\x02cuuid\nUUID\nq\x00)\x81q\x01}q\x02U\x03intq\x03\x8a\x10xV' - b'4\x12xV4\x12xV4\x12xV4\x12sb.', + pickled_uuids = [ + # Python 2.7, protocol 0 + b'ccopy_reg\n_reconstructor\n(cuuid\nUUID\nc__builtin__\nobject\nN' + b'tR(dS\'int\'\nL287307832597519156748809049798316161701L\nsb.', + # Python 2.7, protocol 1 + b'ccopy_reg\n_reconstructor\n(cuuid\nUUID\nc__builtin__\nobject\nN' + b'tR}U\x03intL287307832597519156748809049798316161701L\nsb.', + # Python 2.7, protocol 2 + b'\x80\x02cuuid\nUUID\n)\x81}U\x03int\x8a\x11\xa5z\xecz\nI\xdf}' + b'\xde\xa0Bf\xcey%\xd8\x00sb.', + # Python 3.6, protocol 0 + b'ccopy_reg\n_reconstructor\n(cuuid\nUUID\nc__builtin__\nobject\nN' + b'tR(dVint\nL287307832597519156748809049798316161701L\nsb.', + # Python 3.6, protocol 1 + b'ccopy_reg\n_reconstructor\n(cuuid\nUUID\nc__builtin__\nobject\nN' + b'tR}X\x03\x00\x00\x00intL287307832597519156748809049798316161701L' + b'\nsb.', + # Python 3.6, protocol 2 + b'\x80\x02cuuid\nUUID\n)\x81}X\x03\x00\x00\x00int\x8a\x11\xa5z\xec' + b'z\nI\xdf}\xde\xa0Bf\xcey%\xd8\x00sb.', + # Python 3.6, protocol 3 + b'\x80\x03cuuid\nUUID\n)\x81}X\x03\x00\x00\x00int\x8a\x11\xa5z\xec' + b'z\nI\xdf}\xde\xa0Bf\xcey%\xd8\x00sb.', + # Python 3.6, protocol 4 + b'\x80\x04\x95+\x00\x00\x00\x00\x00\x00\x00\x8c\x04uuid\x8c\x04UUI' + b'D\x93)\x81}\x8c\x03int\x8a\x11\xa5z\xecz\nI\xdf}\xde\xa0Bf\xcey%' + b'\xd8\x00sb.', + # Python 3.7, protocol 0 + b'ccopy_reg\n_reconstructor\n(cuuid\nUUID\nc__builtin__\nobject\nN' + b'tR(dVint\nL287307832597519156748809049798316161701L\nsVis_safe\n' + b'cuuid\nSafeUUID\n(NtRsb.', + # Python 3.7, protocol 1 + b'ccopy_reg\n_reconstructor\n(cuuid\nUUID\nc__builtin__\nobject\nN' + b'tR}(X\x03\x00\x00\x00intL287307832597519156748809049798316161701' + b'L\nX\x07\x00\x00\x00is_safecuuid\nSafeUUID\n(NtRub.', + # Python 3.7, protocol 2 + b'\x80\x02cuuid\nUUID\n)\x81}(X\x03\x00\x00\x00int\x8a\x11\xa5z' + b'\xecz\nI\xdf}\xde\xa0Bf\xcey%\xd8\x00X\x07\x00\x00\x00is_safecuu' + b'id\nSafeUUID\nN\x85Rub.', + # Python 3.7, protocol 3 + b'\x80\x03cuuid\nUUID\n)\x81}(X\x03\x00\x00\x00int\x8a\x11\xa5z' + b'\xecz\nI\xdf}\xde\xa0Bf\xcey%\xd8\x00X\x07\x00\x00\x00is_safecuu' + b'id\nSafeUUID\nN\x85Rub.', + # Python 3.7, protocol 4 + b'\x80\x04\x95F\x00\x00\x00\x00\x00\x00\x00\x8c\x04uuid\x94\x8c' + b'\x04UUID\x93)\x81}(\x8c\x03int\x8a\x11\xa5z\xecz\nI\xdf}\xde\xa0' + b'Bf\xcey%\xd8\x00\x8c\x07is_safeh\x00\x8c\x08SafeUUID\x93N\x85Rub' + b'.', ] - # Python 3.6 protocol 0-4 pickles of u - py36_pickles = [ - b'ccopy_reg\n_reconstructor\np0\n(cuuid\nUUID\np1\nc__builtin__\nob' - b'ject\np2\nNtp3\nRp4\n(dp5\nVint\np6\nL241978571610117151621718396' - b'36988778104L\nsb.', - b'ccopy_reg\n_reconstructor\nq\x00(cuuid\nUUID\nq\x01c__builtin__\n' - b'object\nq\x02Ntq\x03Rq\x04}q\x05X\x03\x00\x00\x00intq\x06L2419785' - b'7161011715162171839636988778104L\nsb.', - b'\x80\x02cuuid\nUUID\nq\x00)\x81q\x01}q\x02X\x03\x00\x00\x00intq' - b'\x03\x8a\x10xV4\x12xV4\x12xV4\x12xV4\x12sb.', - b'\x80\x03cuuid\nUUID\nq\x00)\x81q\x01}q\x02X\x03\x00\x00\x00intq' - b'\x03\x8a\x10xV4\x12xV4\x12xV4\x12xV4\x12sb.', - b'\x80\x04\x950\x00\x00\x00\x00\x00\x00\x00\x8c\x04uuid\x94\x8c\x04' - b'UUID\x94\x93\x94)\x81\x94}\x94\x8c\x03int\x94\x8a\x10xV4\x12xV4' - b'\x12xV4\x12xV4\x12sb.', + pickled_uuids_safe = [ + # Python 3.7, protocol 0 + b'ccopy_reg\n_reconstructor\n(cuuid\nUUID\nc__builtin__\nobject\nN' + b'tR(dVint\nL287307832597519156748809049798316161701L\nsVis_safe\n' + b'cuuid\nSafeUUID\n(I0\ntRsb.', + # Python 3.7, protocol 1 + b'ccopy_reg\n_reconstructor\n(cuuid\nUUID\nc__builtin__\nobject\nN' + b'tR}(X\x03\x00\x00\x00intL287307832597519156748809049798316161701' + b'L\nX\x07\x00\x00\x00is_safecuuid\nSafeUUID\n(K\x00tRub.', + # Python 3.7, protocol 2 + b'\x80\x02cuuid\nUUID\n)\x81}(X\x03\x00\x00\x00int\x8a\x11\xa5z' + b'\xecz\nI\xdf}\xde\xa0Bf\xcey%\xd8\x00X\x07\x00\x00\x00is_safecuu' + b'id\nSafeUUID\nK\x00\x85Rub.', + # Python 3.7, protocol 3 + b'\x80\x03cuuid\nUUID\n)\x81}(X\x03\x00\x00\x00int\x8a\x11\xa5z' + b'\xecz\nI\xdf}\xde\xa0Bf\xcey%\xd8\x00X\x07\x00\x00\x00is_safecuu' + b'id\nSafeUUID\nK\x00\x85Rub.', + # Python 3.7, protocol 4 + b'\x80\x04\x95G\x00\x00\x00\x00\x00\x00\x00\x8c\x04uuid\x94\x8c' + b'\x04UUID\x93)\x81}(\x8c\x03int\x8a\x11\xa5z\xecz\nI\xdf}\xde\xa0' + b'Bf\xcey%\xd8\x00\x8c\x07is_safeh\x00\x8c\x08SafeUUID\x93K\x00' + b'\x85Rub.', + ] + pickled_uuids_unsafe = [ + # Python 3.7, protocol 0 + b'ccopy_reg\n_reconstructor\n(cuuid\nUUID\nc__builtin__\nobject\nN' + b'tR(dVint\nL287307832597519156748809049798316161701L\nsVis_safe\n' + b'cuuid\nSafeUUID\n(I-1\ntRsb.', + # Python 3.7, protocol 1 + b'ccopy_reg\n_reconstructor\n(cuuid\nUUID\nc__builtin__\nobject\nN' + b'tR}(X\x03\x00\x00\x00intL287307832597519156748809049798316161701' + b'L\nX\x07\x00\x00\x00is_safecuuid\nSafeUUID\n(J\xff\xff\xff\xfftR' + b'ub.', + # Python 3.7, protocol 2 + b'\x80\x02cuuid\nUUID\n)\x81}(X\x03\x00\x00\x00int\x8a\x11\xa5z' + b'\xecz\nI\xdf}\xde\xa0Bf\xcey%\xd8\x00X\x07\x00\x00\x00is_safecuu' + b'id\nSafeUUID\nJ\xff\xff\xff\xff\x85Rub.', + # Python 3.7, protocol 3 + b'\x80\x03cuuid\nUUID\n)\x81}(X\x03\x00\x00\x00int\x8a\x11\xa5z' + b'\xecz\nI\xdf}\xde\xa0Bf\xcey%\xd8\x00X\x07\x00\x00\x00is_safecuu' + b'id\nSafeUUID\nJ\xff\xff\xff\xff\x85Rub.', + # Python 3.7, protocol 4 + b'\x80\x04\x95J\x00\x00\x00\x00\x00\x00\x00\x8c\x04uuid\x94\x8c' + b'\x04UUID\x93)\x81}(\x8c\x03int\x8a\x11\xa5z\xecz\nI\xdf}\xde\xa0' + b'Bf\xcey%\xd8\x00\x8c\x07is_safeh\x00\x8c\x08SafeUUID\x93J\xff' + b'\xff\xff\xff\x85Rub.', ] - for pickled in py27_pickles + py36_pickles: - unpickled = pickle.loads(pickled) - self.assertEqual(unpickled, u) - # is_safe was added in 3.7. When unpickling values from older - # versions, is_safe will be missing, so it should be set to - # SafeUUID.unknown. - self.assertEqual(unpickled.is_safe, self.uuid.SafeUUID.unknown) + u = self.uuid.UUID('d82579ce6642a0de7ddf490a7aec7aa5') + u_safe = self.uuid.UUID('d82579ce6642a0de7ddf490a7aec7aa5', + is_safe=self.uuid.SafeUUID.safe) + u_unsafe = self.uuid.UUID('d82579ce6642a0de7ddf490a7aec7aa5', + is_safe=self.uuid.SafeUUID.unsafe) + + with support.swap_item(sys.modules, 'uuid', self.uuid): + for pickled in pickled_uuids: + # is_safe was added in 3.7. When unpickling values from older + # versions, is_safe will be missing, so it should be set to + # SafeUUID.unknown. + check(pickle.loads(pickled), u) + for pickled in pickled_uuids_safe: + check(pickle.loads(pickled), u_safe) + for pickled in pickled_uuids_unsafe: + check(pickle.loads(pickled), u_unsafe) # bpo-32502: UUID1 requires a 48-bit identifier, but hardware identifiers # need not necessarily be 48 bits (e.g., EUI-64). diff --git a/Lib/uuid.py b/Lib/uuid.py index 51538822151..073ca711ab4 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -207,26 +207,19 @@ class UUID: object.__setattr__(self, 'is_safe', is_safe) def __getstate__(self): - d = {attr: getattr(self, attr) for attr in self.__slots__} - # is_safe is a SafeUUID instance. Return just its value, so that - # it can be unpickled in older Python versions without SafeUUID. - d['is_safe'] = d['is_safe'].value + d = {'int': self.int} + if self.is_safe != SafeUUID.unknown: + # is_safe is a SafeUUID instance. Return just its value, so that + # it can be un-pickled in older Python versions without SafeUUID. + d['is_safe'] = self.is_safe.value return d def __setstate__(self, state): - # is_safe was added in 3.7 - state.setdefault('is_safe', SafeUUID.unknown.value) - - for attr in self.__slots__: - value = state[attr] - - # for is_safe, restore the SafeUUID from the stored value - if attr == 'is_safe': - try: - value = SafeUUID(value) - except ValueError: - value = SafeUUID.unknown - object.__setattr__(self, attr, value) + object.__setattr__(self, 'int', state['int']) + # is_safe was added in 3.7; it is also omitted when it is "unknown" + object.__setattr__(self, 'is_safe', + SafeUUID(state['is_safe']) + if 'is_safe' in state else SafeUUID.unknown) def __eq__(self, other): if isinstance(other, UUID):