Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
bpo-43574: Dont overallocate list literals#24954
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
4dbee9a5b31470172f2c0d76ae2a7aad2467c20c6c7be3ae87436223657d51f4336cc619d4374File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -2,6 +2,7 @@ | ||
| from test import list_tests | ||
| from test.support import cpython_only | ||
| import pickle | ||
| import struct | ||
| import unittest | ||
| class ListTest(list_tests.CommonTest): | ||
| @@ -196,6 +197,63 @@ def test_preallocation(self): | ||
| self.assertEqual(iter_size, sys.getsizeof(list([0] * 10))) | ||
| self.assertEqual(iter_size, sys.getsizeof(list(range(10)))) | ||
| @cpython_only | ||
| def test_overallocation(self): | ||
| # bpo-33234: Don't overallocate when initialized from known lengths | ||
| # bpo-38373: Allows list over-allocation to be zero for some lengths | ||
| # bpo-43574: Don't overallocate for list-literals | ||
| sizeof = sys.getsizeof | ||
| # First handle empty list and empty list-literal cases. Should have no | ||
| # overallocation, including init from iterable of unknown length. | ||
| self.assertEqual(sizeof([]), sizeof(list())) | ||
| self.assertEqual(sizeof([]), sizeof(list(tuple()))) | ||
| self.assertEqual(sizeof([]), sizeof(list(x for x in []))) | ||
| # Must use actual list-literals to test the overallocation behavior of | ||
| # compiled list-literals as well as those initialized from them. | ||
| test_literals = [ | ||
| [1], | ||
| [1,2], | ||
| [1,2,3], # Literals of length > 2 are special-cased in compile | ||
| [1,2,3,4], | ||
| [1,2,3,4,5,6,7], | ||
| [1,2,3,4,5,6,7,8], # bpo-38373: Length 8 init won't over-alloc | ||
| [1,2,3,4,5,6,7,8,9], | ||
| ] | ||
| overalloc_amts = [] | ||
| for literal in test_literals: | ||
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. no action required: meta: this is where I really wish we had an 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. I think unittest.subTest() displays parameters on the failing subtests and keeps running through each even on failure. | ||
| # Direct check that list-literals do not over-allocate, by | ||
| # calculating the total size of used pointers. | ||
| total_ptr_size = len(literal) * struct.calcsize('P') | ||
| self.assertEqual(sizeof(literal), sizeof([]) + total_ptr_size) | ||
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. add a | ||
| # Ensure that both list literals, and lists made from an iterable | ||
| # of known size, use the same amount of allocation. | ||
| self.assertEqual(sizeof(literal), sizeof(list(literal))) | ||
| self.assertEqual(sizeof(literal), sizeof(list(tuple(literal)))) | ||
| # By contrast, confirm that non-empty lists initialized from an | ||
| # iterable where the length is unknown at the time of | ||
| # initialization, can be overallocated. | ||
| iterated_list = list(x for x in literal) | ||
| overalloc_amts.append(sizeof(iterated_list) - sizeof(literal)) | ||
| self.assertGreaterEqual(sizeof(iterated_list), sizeof(literal)) | ||
| # bpo-38373: initialized or grown lists are not always over-allocated. | ||
| # Confirm that over-allocation occurs at least some of the time. | ||
| self.assertEqual(True, any(x>0 for x in overalloc_amts)) | ||
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. add | ||
| # Empty lists should overallocate on initial append/insert (unlike | ||
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. Turn this bit into an additional test method. | ||
| # list-literals) | ||
| l1 = [] | ||
| l1.append(1) | ||
| self.assertGreater(sizeof(l1), sizeof([1])) | ||
| l2 = [] | ||
| l2.insert(0, 1) | ||
| self.assertGreater(sizeof(l2), sizeof([1])) | ||
| def test_count_index_remove_crashes(self): | ||
| # bpo-38610: The count(), index(), and remove() methods were not | ||
| # holding strong references to list elements while calling | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| ``list`` objects don't overallocate when starting empty and then extended, or | ||
| when set to be empty. This effectively restores previous ``list`` memory | ||
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. quantify what "previous" means. 3.8 and earlier I believe. | ||
| behavior for lists initialized from literals. | ||
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.
make this set of initial assertions into its own test method.