bpo-33234 Improve list() pre-sizing for inputs with known lengths (no __length_hint__) #9846

pablogsal · 2018-10-13T17:01:37Z

This is a version of #6493 that only preallocates the list when __len__ is available to reduce memory overallocation and possible performance hit on big iterables.

This are the L1 and L2 cache misses and related information:

THIS PR

❯ perf stat -r 200 -B -e cache-references,cache-misses,cycles,instructions,branches,faults,migrations ./python -c "
for _ in range(1000):
    list([0]*10000)
"
 Performance counter stats for './python -c
for _ in range(1000):
    list([0]*10000)
' (200 runs):

         1,151,365      cache-references:u                                            ( +-  0.47% )
             5,686      cache-misses:u            #    0.494 % of all cache refs      ( +-  8.06% )
       370,237,437      cycles:u                                                      ( +-  0.05% )
       542,049,270      instructions:u            #    1.46  insn per cycle           ( +-  0.00% )
       134,146,925      branches:u                                                    ( +-  0.00% )
             8,031      faults:u                                                      ( +-  0.00% )
                 0      migrations:u

          0.122170 +- 0.000307 seconds time elapsed  ( +-  0.25% )

MASTER

❯ perf stat -r 200 -B -e cache-references,cache-misses,cycles,instructions,branches,faults,migrations ./python -c "for _ in range(1000):
    list([0]*10000)
"
 Performance counter stats for './python -c
for _ in range(1000):
    list([0]*10000)
' (200 runs):

         1,275,440      cache-references:u                                            ( +-  0.61% )
            21,895      cache-misses:u            #    1.717 % of all cache refs      ( +-  4.69% )
       392,293,096      cycles:u                                                      ( +-  0.22% )
       543,254,939      instructions:u            #    1.38  insn per cycle           ( +-  0.00% )
       134,549,173      branches:u                                                    ( +-  0.00% )
            10,039      faults:u                                                      ( +-  0.00% )
                 0      migrations:u

          0.133171 +- 0.000463 seconds time elapsed  ( +-  0.35% )

https://bugs.python.org/issue33234

The list() constructor isn't taking full advantage of known input lengths or length hints. This commit makes the constructor pre-size and not over-allocate when the input size is known or can be reasonably estimated. A new test is added to test_list.py and a test needed to be modified in test_descr.py as it assumed that there is only one call to __length_hint__() in the list constructor.

…ce for small iterables

vstinner

LGTM, but the NEWS entry should be fixed.

vstinner · 2018-10-15T20:56:11Z

Misc/NEWS.d/next/Core and Builtins/2018-04-17-01-24-51.bpo-33234.l9IDtp.rst

@@ -0,0 +1,2 @@
+The list constructor will pre-size and not over-allocate when the input size


length, not size.

vstinner · 2018-10-15T20:56:39Z

Misc/NEWS.d/next/Core and Builtins/2018-04-17-01-24-51.bpo-33234.l9IDtp.rst

@@ -0,0 +1,2 @@
+The list constructor will pre-size and not over-allocate when the input size
+is known or can be reasonably estimated.


"or can be reasonably estimated" I don't understand this part.

vstinner · 2018-10-15T20:58:51Z

Objects/listobject.c

@@ -2649,6 +2675,13 @@ list___init___impl(PyListObject *self, PyObject *iterable)
        (void)_list_clear(self);
    }
    if (iterable != NULL) {
+        Py_ssize_t iter_len = PyObject_Length(iterable);
+        if (iter_len == -1){


nitpick: ...) { (add a space)

vstinner

LGTM.

vstinner · 2018-10-15T21:53:46Z

I suggest to wait the approval of a second core dev before merging this change.

pablogsal · 2018-10-15T22:17:46Z

I would like @serhiy-storchaka comment and advice on this if possible.

vstinner · 2018-10-23T10:16:32Z

@pablogsal: If you want me to merge your PR, I would be me confident if you write a benchmark for the worst case: a small container (ex: 10 items or less) without a known length. I expect that the additional function call (obj.len()) would slow down this case. I would be interested to see the overhead.

pablogsal · 2018-10-23T12:09:03Z

@vstinner

Experiment:

import perf

runner = perf.Runner()
runner.timeit("list(x)",
              stmt="list(x)",
              setup="x = iter(list(range(10)))")

result:

 ./python.exe -m perf  compare_to old.json new.json
Mean +- std dev: [old] 795 ns +- 25 ns -> [new] 2.94 us +- 0.07 us: 3.70x slower (+270%)

With the experiment:

import perf

runner = perf.Runner()
runner.timeit("list(x)",
              stmt="list(x)",
              setup="x = [0]*10000")

the result is

❯ ./python.exe -m perf  compare_to old_big.json new_big.json
Mean +- std dev: [old_big] 97.5 us +- 3.4 us -> [new_big] 81.4 us +- 3.7 us: 1.19x faster (-5%)

I would say that this result points to abandon this version of the change, given the bad performance for small iterables.

pablogsal · 2018-10-23T12:20:46Z

This is the first experiment using PyObject_LengthHint:

❯ ./python.exe -m perf  compare_to old.json new.json
Mean +- std dev: [old] 795 ns +- 25 ns -> [new] 882 ns +- 32 ns: 1.11x slower (+11%)

vstinner · 2018-10-23T12:46:32Z

Mean +- std dev: [old] 795 ns +- 25 ns -> [new] 2.94 us +- 0.07 us: 3.70x slower (+270%)

This overhead likely comes from the raisin an exception + PyErr_Clear(). hasattr() has been optimized by adding a _PyObject_LookupAttr() which is getattr() without raising an exception if the attribute doesn't exist.

Probing an object size is a common need for optimization to preallocate a container. Python builtin types would benefit of such function.

What do you think of adding something like _PyObject_ProbeLength() which is like PyObject_Length() without raising an exception?

cc @methane @serhiy-storchaka

vstinner · 2018-10-23T12:48:16Z

Objects/listobject.c

@@ -2649,6 +2675,13 @@ list___init___impl(PyListObject *self, PyObject *iterable)
        (void)_list_clear(self);
    }
    if (iterable != NULL) {
+        Py_ssize_t iter_len = PyObject_Length(iterable);


To reduce or avoid the overhead of the worst case, you can avoid this call if _PyObject_HasLen() is false.

This change doesn't require to implement a new _PyObject_ProbeLength().

Updated run of first experiment using _PyObject_HasLen():

Mean +- std dev: [old] 795 ns +- 25 ns -> [new] 776 ns +- 25 ns: 1.02x faster (-6%)

pablogsal · 2018-10-23T12:57:32Z

Updated run of first experiment using _PyObject_HasLen():

❯ ./python.exe -m perf  compare_to old.json new.json
Mean +- std dev: [old] 795 ns +- 25 ns -> [new] 776 ns +- 25 ns: 1.02x faster (-6%)

vstinner · 2018-10-23T17:23:27Z

Updated run of first experiment using _PyObject_HasLen():

If it's the "x = iter(list(range(10)))" worst case: it confirms what I expected, _PyObject_HasLen() makes the overhead on the worst case not significant. Good.

I guess that you kept the same speedup for the best case?

serhiy-storchaka · 2018-10-23T18:06:11Z

Objects/listobject.c

@@ -2649,6 +2675,15 @@ list___init___impl(PyListObject *self, PyObject *iterable)
        (void)_list_clear(self);
    }
    if (iterable != NULL) {
+        if(_PyObject_HasLen(iterable)){


Add a space before {.

Objects/listobject.c

serhiy-storchaka · 2018-10-23T18:13:00Z

Objects/listobject.c

@@ -2649,6 +2675,15 @@ list___init___impl(PyListObject *self, PyObject *iterable)
        (void)_list_clear(self);
    }
    if (iterable != NULL) {
+        if(_PyObject_HasLen(iterable)){
+            Py_ssize_t iter_len = PyObject_Length(iterable);


PyObject_Length is an alias of PyObject_Size (for backward compatibility). It is better to use PyObject_Size.

Objects/listobject.c

serhiy-storchaka · 2018-10-23T18:22:56Z

Objects/listobject.c

+        allocated = 0;
+    }
+    num_allocated_bytes = allocated * sizeof(PyObject *);
+    items = (PyObject **)PyMem_Malloc(num_allocated_bytes);


Would not be easier to use PyMem_New?

pablogsal · 2018-10-23T20:08:58Z

Performance metrics with commit b678442:

❯ ./python -m perf compare_to ../cpython2/old.json new.json
Mean +- std dev: [old_big] 97.5 us +- 3.4 us -> [new_big] 88.6 us +- 5.5 us: 1.1x faster (-5%)

and perf information:

❯ perf stat -r 200 -B -e cache-references,cache-misses,cycles,instructions,branches,faults,migrations ./python -c "
for _ in range(1000):
    list([0]*10000)
"
 Performance counter stats for './python -c
for _ in range(1000):
    list([0]*10000)
' (200 runs):

         1,151,607      cache-references:u                                            ( +-  0.51% )
             3,438      cache-misses:u            #    0.299 % of all cache refs      ( +- 14.03% )
       379,755,424      cycles:u                                                      ( +-  0.56% )
       543,025,748      instructions:u            #    1.43  insn per cycle           ( +-  0.00% )
       134,399,489      branches:u                                                    ( +-  0.00% )
             8,029      faults:u                                                      ( +-  0.00% )
                 0      migrations:u

          0.123801 +- 0.000686 seconds time elapsed  ( +-  0.55% )

Also, let's not forget the original objective of this patch:

PATCH

>>> import sys
>>> x = list([0]*1000000)
>>>
KeyboardInterrupt
>>> sys.getsizeof(x)
8000080

MASTER

>>> import sys
>>> x = list([0]*1000000)
>>>
KeyboardInterrupt
>>> sys.getsizeof(x)
9000120

This is a 12 % improvement in memory claimed by the list in these cases.

methane

LGTM

vstinner

LGTM, it's now even better with _PyObject_HasLen()!

Objects/listobject.c

vstinner · 2018-10-25T11:06:46Z

Objects/listobject.c

+            if (iter_len == -1) {
+                if (PyErr_ExceptionMatches(PyExc_Exception)) {
+                    PyErr_Clear();
+                } else {


nitpick: PEP 7 requires

} else {

serhiy-storchaka · 2018-10-26T05:11:25Z

Objects/listobject.c

+        if (_PyObject_HasLen(iterable) && self->ob_item == NULL) {
+            Py_ssize_t iter_len = PyObject_Size(iterable);
+            if (iter_len == -1) {
+                if (PyErr_ExceptionMatches(PyExc_Exception)) {


What tests will fail if not silence any exceptions at all?

At least the following:

test_generators failed ********************************************************************** File "/home/pablogsal/github/cpython/Lib/test/test_generators.py", line ?, in test.test_generators.__test__.weakref Failed example: list(p) Exception raised: Traceback (most recent call last): File "/home/pablogsal/github/cpython/Lib/doctest.py", line 1329, in __run compileflags, 1), test.globs) File "<doctest test.test_generators.__test__.weakref[9]>", line 1, in <module> list(p) TypeError: object of type 'generator' has no len() ********************************************************************** 1 items had failures: 1 of 10 in test.test_generators.__test__.weakref ***Test Failed*** 1 failures. test_genexps failed ********************************************************************** File "/home/pablogsal/github/cpython/Lib/test/test_genexps.py", line ?, in test.test_genexps.__test__.doctests Failed example: list(p) Exception raised: Traceback (most recent call last): File "/home/pablogsal/github/cpython/Lib/doctest.py", line 1329, in __run compileflags, 1), test.globs) File "<doctest test.test_genexps.__test__.doctests[75]>", line 1, in <module> list(p) TypeError: object of type 'generator' has no len() ********************************************************************** 1 items had failures: 1 of 76 in test.test_genexps.__test__.doctests ***Test Failed*** 1 failures.

How can _PyObject_HasLen() return true for generators?

Ah, it is not true generator, it is a weakref. What if silence just TypeError?

I am a bit afraid if someone has similar tests/usage like these two cases and then they will receive new errors as raising all exceptions would be technically a backwards incompatible change. But if you think we should raise (maybe ignoring TypeError) I am happy to change the implementation.

See PyObject_LengthHint().

Done in 0a6c8ba

Objects/listobject.c

vstinner · 2018-10-26T11:28:38Z

Objects/listobject.c

+                if (PyErr_ExceptionMatches(PyExc_Exception)) {
+                    PyErr_Clear();
+                }
+                else {


nitpick: for readability, I suggest to invert the if condition and exchange the true/false blocks, and move PyErr_Clear() after the if:

if (!not exception) { return -1; }
PyErr_Clear();

vstinner

LGTM.

vstinner · 2018-10-28T15:44:42Z

Objects/listobject.c

+                if (!PyErr_ExceptionMatches(PyExc_TypeError)) {
+                    return -1;
+                }
+                PyErr_Clear();


@pablogsal, @serhiy-storchaka: In a previous comment, I proposed to add an helper function to "probe" an object size: so move this code into a private helper function. Since the same code is used by PyObject_LengthHint(), it would now make sense, no?

See also iter_len() ... which is different.

This code will be used just in two places and is not too complex. Adding yet one intermediate function will add a performance penalty for calling a function and checking its result, and will complicate the code. If this code will be used in more places, it can be refactored.

pablogsal added 2 commits April 17, 2018 21:51

Preallocate only when __len__ is available to avoid loss of performan…

39a8757

…ce for small iterables

the-knights-who-say-ni added the CLA signed label Oct 13, 2018

bedevere-bot added the awaiting merge label Oct 13, 2018

pablogsal requested review from vstinner, rhettinger and serhiy-storchaka October 13, 2018 17:01

vstinner approved these changes Oct 15, 2018

View reviewed changes

Fix NEWS entry and correct style

ab450b3

vstinner approved these changes Oct 15, 2018

View reviewed changes

pablogsal added the DO-NOT-MERGE label Oct 23, 2018

vstinner reviewed Oct 23, 2018

View reviewed changes

Use _PyObject_HasLen to check if the object has lenght to avoid overhead

717ca67

serhiy-storchaka reviewed Oct 23, 2018

View reviewed changes

Simplify implementation and don't capture all expections on error

b678442

methane approved these changes Oct 25, 2018

View reviewed changes

vstinner approved these changes Oct 25, 2018

View reviewed changes

pablogsal added 2 commits October 25, 2018 22:36

Add assert and fix formatting

b40bd62

Remove uneeded test modification

3837486

serhiy-storchaka reviewed Oct 26, 2018

View reviewed changes

pablogsal force-pushed the bpo33234-2 branch from d99098c to 3837486 Compare October 26, 2018 09:30

Don't preallocate if list is modified when calling PyObject_Size

b6ff7ce

vstinner reviewed Oct 26, 2018

View reviewed changes

Improve readability in the error handling branch

e7f8c46

pablogsal removed the DO-NOT-MERGE label Oct 26, 2018

vstinner approved these changes Oct 26, 2018

View reviewed changes

Capture only TypeError as suggested by Serhiy

0a6c8ba

vstinner reviewed Oct 28, 2018

View reviewed changes

serhiy-storchaka approved these changes Oct 28, 2018

View reviewed changes

pablogsal merged commit 372d705 into python:master Oct 28, 2018

bedevere-bot removed the awaiting merge label Oct 28, 2018

pablogsal deleted the bpo33234-2 branch October 28, 2018 20:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-33234 Improve list() pre-sizing for inputs with known lengths (no __length_hint__) #9846

bpo-33234 Improve list() pre-sizing for inputs with known lengths (no __length_hint__) #9846

pablogsal commented Oct 13, 2018 •

edited

Loading

vstinner left a comment

vstinner Oct 15, 2018

vstinner Oct 15, 2018

vstinner Oct 15, 2018

vstinner left a comment

vstinner commented Oct 15, 2018

pablogsal commented Oct 15, 2018

vstinner commented Oct 23, 2018

pablogsal commented Oct 23, 2018 •

edited

Loading

pablogsal commented Oct 23, 2018 •

edited

Loading

vstinner commented Oct 23, 2018

vstinner Oct 23, 2018

pablogsal Oct 23, 2018

pablogsal commented Oct 23, 2018 •

edited

Loading

vstinner commented Oct 23, 2018

serhiy-storchaka Oct 23, 2018

serhiy-storchaka Oct 23, 2018

serhiy-storchaka Oct 23, 2018

pablogsal commented Oct 23, 2018 •

edited

Loading

methane left a comment

vstinner left a comment

vstinner Oct 25, 2018

serhiy-storchaka Oct 26, 2018

pablogsal Oct 26, 2018 •

edited

Loading

serhiy-storchaka Oct 27, 2018

serhiy-storchaka Oct 27, 2018

pablogsal Oct 27, 2018

serhiy-storchaka Oct 28, 2018

pablogsal Oct 28, 2018

vstinner Oct 26, 2018

vstinner left a comment

vstinner Oct 28, 2018

serhiy-storchaka Oct 28, 2018

		@@ -0,0 +1,2 @@
		The list constructor will pre-size and not over-allocate when the input size

		@@ -0,0 +1,2 @@
		The list constructor will pre-size and not over-allocate when the input size
		is known or can be reasonably estimated.

bpo-33234 Improve list() pre-sizing for inputs with known lengths (no __length_hint__) #9846

bpo-33234 Improve list() pre-sizing for inputs with known lengths (no __length_hint__) #9846

Conversation

pablogsal commented Oct 13, 2018 • edited Loading

vstinner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vstinner left a comment

Choose a reason for hiding this comment

vstinner commented Oct 15, 2018

pablogsal commented Oct 15, 2018

vstinner commented Oct 23, 2018

pablogsal commented Oct 23, 2018 • edited Loading

pablogsal commented Oct 23, 2018 • edited Loading

vstinner commented Oct 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pablogsal commented Oct 23, 2018 • edited Loading

vstinner commented Oct 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pablogsal commented Oct 23, 2018 • edited Loading

PATCH

MASTER

methane left a comment

Choose a reason for hiding this comment

vstinner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pablogsal Oct 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vstinner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pablogsal commented Oct 13, 2018 •

edited

Loading

pablogsal commented Oct 23, 2018 •

edited

Loading

pablogsal commented Oct 23, 2018 •

edited

Loading

pablogsal commented Oct 23, 2018 •

edited

Loading

pablogsal commented Oct 23, 2018 •

edited

Loading

pablogsal Oct 26, 2018 •

edited

Loading