[mypyc] Implement list insert primitive #9741

vsakkas · 2020-11-21T11:52:14Z

Description

Implements list insert primitive for improved performance.

Test Plan

Ensure that inserting at the start, middle and end of a list works as expected, check performance improvements.

Generated IR

The following script was used:

from typing import List
l : List[int] = []
l.insert(0, 1)

Master branch:

def __top_level__():
    r0, r1 :: object
    r2 :: bit
    r3 :: str
    r4, r5, r6 :: object
    r7 :: bit
    r8 :: str
    r9, r10 :: object
    r11 :: dict
    r12 :: str
    r13 :: object
    r14 :: str
    r15 :: int32
    r16 :: bit
    r17 :: list
    r18 :: dict
    r19 :: str
    r20 :: int32
    r21 :: bit
    r22 :: dict
    r23 :: str
    r24 :: object
    r25 :: list
    r26 :: str
    r27, r28, r29 :: object
    r30 :: None
L0:
    r0 = builtins :: module
    r1 = load_address _Py_NoneStruct
    r2 = r0 != r1
    if r2 goto L3 else goto L1 :: bool
L1:
    r3 = load_global CPyStatic_unicode_0 :: static  ('builtins')
    r4 = PyImport_Import(r3)
    if is_error(r4) goto L14 (error at <module>:-1) else goto L2
L2:
    builtins = r4 :: module
    dec_ref r4
L3:
    r5 = typing :: module
    r6 = load_address _Py_NoneStruct
    r7 = r5 != r6
    if r7 goto L6 else goto L4 :: bool
L4:
    r8 = load_global CPyStatic_unicode_1 :: static  ('typing')
    r9 = PyImport_Import(r8)
    if is_error(r9) goto L14 (error at <module>:1) else goto L5
L5:
    typing = r9 :: module
    dec_ref r9
L6:
    r10 = typing :: module
    r11 = program.globals :: static
    r12 = load_global CPyStatic_unicode_2 :: static  ('List')
    r13 = CPyObject_GetAttr(r10, r12)
    if is_error(r13) goto L14 (error at <module>:1) else goto L7
L7:
    r14 = load_global CPyStatic_unicode_2 :: static  ('List')
    r15 = CPyDict_SetItem(r11, r14, r13)
    dec_ref r13
    r16 = r15 >= 0 :: signed
    if not r16 goto L14 (error at <module>:1) else goto L8 :: bool
L8:
    r17 = PyList_New(0)
    if is_error(r17) goto L14 (error at <module>:2) else goto L9
L9:
    r18 = program.globals :: static
    r19 = load_global CPyStatic_unicode_3 :: static  ('l')
    r20 = CPyDict_SetItem(r18, r19, r17)
    dec_ref r17
    r21 = r20 >= 0 :: signed
    if not r21 goto L14 (error at <module>:2) else goto L10 :: bool
L10:
    r22 = program.globals :: static
    r23 = load_global CPyStatic_unicode_3 :: static  ('l')
    r24 = CPyDict_GetItem(r22, r23)
    if is_error(r24) goto L14 (error at <module>:3) else goto L11
L11:
    r25 = cast(list, r24)
    if is_error(r25) goto L14 (error at <module>:3) else goto L12
L12:
    r26 = load_global CPyStatic_unicode_4 :: static  ('insert')
    r27 = box(short_int, 0)
    r28 = box(short_int, 2)
    r29 = CPyObject_CallMethodObjArgs(r25, r26, r27, r28, 0)
    dec_ref r25
    dec_ref r27
    dec_ref r28
    if is_error(r29) goto L14 (error at <module>:3) else goto L15
L13:
    return 1
L14:
    r30 = <error> :: None
    return r30
L15:
    dec_ref r29
    goto L13

PR:

def __top_level__():
    r0, r1 :: object
    r2 :: bit
    r3 :: str
    r4, r5, r6 :: object
    r7 :: bit
    r8 :: str
    r9, r10 :: object
    r11 :: dict
    r12 :: str
    r13 :: object
    r14 :: str
    r15 :: int32
    r16 :: bit
    r17 :: list
    r18 :: dict
    r19 :: str
    r20 :: int32
    r21 :: bit
    r22 :: dict
    r23 :: str
    r24 :: object
    r25 :: list
    r26 :: object
    r27 :: int
    r28 :: None
L0:
    r0 = builtins :: module
    r1 = load_address _Py_NoneStruct
    r2 = r0 != r1
    if r2 goto L3 else goto L1 :: bool
L1:
    r3 = load_global CPyStatic_unicode_0 :: static  ('builtins')
    r4 = PyImport_Import(r3)
    if is_error(r4) goto L14 (error at <module>:-1) else goto L2
L2:
    builtins = r4 :: module
    dec_ref r4
L3:
    r5 = typing :: module
    r6 = load_address _Py_NoneStruct
    r7 = r5 != r6
    if r7 goto L6 else goto L4 :: bool
L4:
    r8 = load_global CPyStatic_unicode_1 :: static  ('typing')
    r9 = PyImport_Import(r8)
    if is_error(r9) goto L14 (error at <module>:1) else goto L5
L5:
    typing = r9 :: module
    dec_ref r9
L6:
    r10 = typing :: module
    r11 = program.globals :: static
    r12 = load_global CPyStatic_unicode_2 :: static  ('List')
    r13 = CPyObject_GetAttr(r10, r12)
    if is_error(r13) goto L14 (error at <module>:1) else goto L7
L7:
    r14 = load_global CPyStatic_unicode_2 :: static  ('List')
    r15 = CPyDict_SetItem(r11, r14, r13)
    dec_ref r13
    r16 = r15 >= 0 :: signed
    if not r16 goto L14 (error at <module>:1) else goto L8 :: bool
L8:
    r17 = PyList_New(0)
    if is_error(r17) goto L14 (error at <module>:2) else goto L9
L9:
    r18 = program.globals :: static
    r19 = load_global CPyStatic_unicode_3 :: static  ('l')
    r20 = CPyDict_SetItem(r18, r19, r17)
    dec_ref r17
    r21 = r20 >= 0 :: signed
    if not r21 goto L14 (error at <module>:2) else goto L10 :: bool
L10:
    r22 = program.globals :: static
    r23 = load_global CPyStatic_unicode_3 :: static  ('l')
    r24 = CPyDict_GetItem(r22, r23)
    if is_error(r24) goto L14 (error at <module>:3) else goto L11
L11:
    r25 = cast(list, r24)
    if is_error(r25) goto L14 (error at <module>:3) else goto L12
L12:
    r26 = box(short_int, 2)
    r27 = PyList_Insert(r25, 0, r26)
    dec_ref r25
    dec_ref r26
    if is_error(r27) goto L14 (error at <module>:3) else goto L15
L13:
    return 1
L14:
    r28 = <error> :: None
    return r28
L15:
    dec_ref r27 :: int
    goto L13

Performance

Master:

running list_insert
................................................................................................................................................................
interpreted: 0.012250s (avg of 80 iterations; stdev 1%)
compiled:    0.007974s (avg of 80 iterations; stdev 1.7%)

compiled is 1.536x faster

PR:

running list_insert
................................................................................................................................................................
interpreted: 0.012263s (avg of 80 iterations; stdev 1.2%)
compiled:    0.002667s (avg of 80 iterations; stdev 0.98%)

compiled is 4.597x faster

Update

The C helper function has resulted in a small decrease in the performance benefits:

running list_insert
...................................................................................................................................................................
interpreted: 0.012055s (avg of 82 iterations; stdev 0.63%)
compiled:    0.002984s (avg of 82 iterations; stdev 0.81%)

compiled is 4.039x faster

TH3CHARLie

Great performance improvement, left a few comments on function signatures.

TH3CHARLie · 2020-11-21T23:08:09Z

mypyc/lib-rt/CPy.h

@@ -323,6 +323,7 @@ bool CPyList_SetItem(PyObject *list, CPyTagged index, PyObject *value);
 PyObject *CPyList_PopLast(PyObject *obj);
 PyObject *CPyList_Pop(PyObject *obj, CPyTagged index);
 CPyTagged CPyList_Count(PyObject *obj, PyObject *value);
+CPyTagged CPyList_Insert(PyObject *list, CPyTagged index, PyObject *value);


return int here

TH3CHARLie · 2020-11-21T23:08:15Z

mypyc/lib-rt/list_ops.c

@@ -108,6 +108,15 @@ CPyTagged CPyList_Count(PyObject *obj, PyObject *value)
    return list_count((PyListObject *)obj, value);
 }

+CPyTagged CPyList_Insert(PyObject *list, CPyTagged index, PyObject *value)


same as above

TH3CHARLie · 2020-11-21T23:09:26Z

mypyc/primitives/list_ops.py

+    arg_types=[list_rprimitive, int_rprimitive, object_rprimitive],
+    return_type=int_rprimitive,
+    c_function_name='CPyList_Insert',
+    error_kind=ERR_MAGIC)


since the function PyList_Insert returns an int here, it is most reasonable to define the op with ERR_NEG_INT. See the append op in list_ops.py for an example

TH3CHARLie · 2020-11-22T10:19:10Z

mypyc/primitives/list_ops.py

+method_op(
+    name='insert',
+    arg_types=[list_rprimitive, int_rprimitive, object_rprimitive],
+    return_type=int_rprimitive,


the return type should be c_int_rprimitive since it corresponds to the low-level int.

Ah, that makes sense. Should be ok now.

TH3CHARLie

Thanks for the update, looks great!

JukkaL · 2020-11-27T11:40:12Z

mypyc/test-data/run-lists.test

+l.insert(2, 2)
+assert l == [0, 1, 2, 3]
+l.insert(4, 4)
+assert l == [0, 1, 2, 3, 4]


@vsakkas This doesn't test the new primitive, since the code is in driver.py, which doesn't get compiled. Would you mind updating the test case? Also, it would be good to check negative indexes and index underflow and overflow.

Of course. I'll look into updating the tests in a new PR.

vsakkas changed the title ~~Implement list insert primitive~~ [mypyc] Implement list insert primitive Nov 21, 2020

vsakkas marked this pull request as draft November 21, 2020 12:14

vsakkas force-pushed the list-insert branch 2 times, most recently from 8d1534b to 032d073 Compare November 21, 2020 12:42

Implement list insert primitive

c7a15d8

vsakkas force-pushed the list-insert branch from 032d073 to c7a15d8 Compare November 21, 2020 13:33

Implement list insert C helper function

1b7105e

vsakkas marked this pull request as ready for review November 21, 2020 22:13

TH3CHARLie reviewed Nov 21, 2020

View reviewed changes

vsakkas added 2 commits November 22, 2020 10:48

Change list insert C helper return type to int

8a2e0f0

Update list insert error kind to neg int

7d83641

TH3CHARLie reviewed Nov 22, 2020

View reviewed changes

Update list insert return type to C int

a740332

TH3CHARLie approved these changes Nov 24, 2020

View reviewed changes

TH3CHARLie requested a review from JukkaL November 24, 2020 02:23

TH3CHARLie merged commit 109d05e into python:master Nov 27, 2020

JukkaL reviewed Nov 27, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[mypyc] Implement list insert primitive #9741

[mypyc] Implement list insert primitive #9741

vsakkas commented Nov 21, 2020 •

edited

TH3CHARLie left a comment

TH3CHARLie Nov 21, 2020

vsakkas Nov 22, 2020

TH3CHARLie Nov 21, 2020

vsakkas Nov 22, 2020

TH3CHARLie Nov 21, 2020

vsakkas Nov 22, 2020

TH3CHARLie Nov 22, 2020

vsakkas Nov 22, 2020

TH3CHARLie left a comment

JukkaL Nov 27, 2020

vsakkas Nov 27, 2020

[mypyc] Implement list insert primitive #9741

[mypyc] Implement list insert primitive #9741

Conversation

vsakkas commented Nov 21, 2020 • edited

Description

Test Plan

Generated IR

Performance

Update

TH3CHARLie 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

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

TH3CHARLie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vsakkas commented Nov 21, 2020 •

edited