-
-
Notifications
You must be signed in to change notification settings - Fork 34.8k
bpo-30977: make uuid.UUID use __slots__ to reduce its memory footprint #9078
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f44236f
4ecc1cb
13a0dcd
bd997e3
1ffe37f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,8 +4,10 @@ | |
| import contextlib | ||
| import io | ||
| import os | ||
| import pickle | ||
| import shutil | ||
| import subprocess | ||
| import sys | ||
|
|
||
| py_uuid = support.import_fresh_module('uuid', blocked=['_uuid']) | ||
| c_uuid = support.import_fresh_module('uuid', fresh=['_uuid']) | ||
|
|
@@ -311,6 +313,64 @@ def test_getnode(self): | |
| 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() | ||
|
|
||
| u = self.uuid.UUID('12345678123456781234567812345678') | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need to test with different |
||
| self.assertEqual(u, pickle.loads(pickle.dumps(u))) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need to test with all pickle protocols. Need to test shallow and deep copying too, since they use the same protocol. |
||
|
|
||
| def test_unpickle_previous_python_versions(self): | ||
| self._setup_for_pickle() | ||
|
|
||
| u = self.uuid.UUID('12345678123456781234567812345678') | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you use UUID that doesn't contain regular patterns? |
||
|
|
||
| # Python 2.7 protocol 0-2 pickles of u | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be better to pass pickles through |
||
| 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.', | ||
| ] | ||
| # 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.', | ||
| ] | ||
|
|
||
| for pickled in py27_pickles + py36_pickles: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why two lists are used?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To clearly differentiate between those generated with different Python versions, using variable names rather than comments. |
||
| 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) | ||
|
|
||
| # bpo-32502: UUID1 requires a 48-bit identifier, but hardware identifiers | ||
| # need not necessarily be 48 bits (e.g., EUI-64). | ||
| def test_uuid1_eui64(self): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -118,6 +118,8 @@ class UUID: | |
| uuid_generate_time_safe(3). | ||
| """ | ||
|
|
||
| __slots__ = ('int', 'is_safe') | ||
|
|
||
| def __init__(self, hex=None, bytes=None, bytes_le=None, fields=None, | ||
| int=None, version=None, | ||
| *, is_safe=SafeUUID.unknown): | ||
|
|
@@ -201,8 +203,30 @@ def __init__(self, hex=None, bytes=None, bytes_le=None, fields=None, | |
| # Set the version number. | ||
| int &= ~(0xf000 << 64) | ||
| int |= version << 76 | ||
| self.__dict__['int'] = int | ||
| self.__dict__['is_safe'] = is_safe | ||
| object.__setattr__(self, 'int', int) | ||
| object.__setattr__(self, 'is_safe', is_safe) | ||
|
|
||
| def __getstate__(self): | ||
| d = {attr: getattr(self, attr) for attr in self.__slots__} | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why use If |
||
| # is_safe is a SafeUUID instance. Return just its value, so that | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the value portable? Seems there is a problem in 3.7 which should be fixed (this is a different issue).
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure what you mean. The value is one of We have to keep just the value to avoid errors when unpickling in Python versions <3.7, where the |
||
| # it can be unpickled in older Python versions without SafeUUID. | ||
| d['is_safe'] = d['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__: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The code would be simpler if just set |
||
| 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 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why this error is silenced?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You brought up the need to support pickles from future Python versions; those could potentially define new values in the |
||
| object.__setattr__(self, attr, value) | ||
|
|
||
| def __eq__(self, other): | ||
| if isinstance(other, UUID): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| Make uuid.UUID use ``__slots__`` to reduce its memory footprint. Based on | ||
| original patch by Wouter Bolsterlee. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't be easier to use
with support.swap_item(sys.modules, 'uuid', self.uuid):?