Skip to content

Commit

Permalink
[External] [stdlib] Integrate PyObject_SetItem for speedup and erro…
Browse files Browse the repository at this point in the history
…r handling (#48447)

[External] [stdlib] Integrate `PyObject_SetItem` for speedup and error
handling

Following the trend of:
- #3549
- #3583

Again, noticing ~1.4x speedup on basic benchmarks:
```mojo
for _ in range(iterations):
    var idx_a = random_si64(0, 999)
    var idx_b = random_si64(0, 9999)
    var idx_c = random_si64(0, 99999)

    a[idx_a] = idx_a
    b[idx_b] = idx_b
    c[idx_c] = idx_c
```

and improved error handling, preventing crashes.

Co-authored-by: Joshua James Venter <venter.joshua@gmail.com>
Closes #3595
MODULAR_ORIG_COMMIT_REV_ID: 107c0b478171f86869b7cb94e57f5ea59d83e994
  • Loading branch information
jjvraw authored and modularbot committed Oct 4, 2024
1 parent 11e97c8 commit 64ef6d1
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 18 deletions.
20 changes: 20 additions & 0 deletions stdlib/src/python/_cpython.mojo
Original file line number Diff line number Diff line change
Expand Up @@ -1053,6 +1053,26 @@ struct CPython:
self._inc_total_rc()
return r

fn PyObject_SetItem(
inout self, obj: PyObjectPtr, key: PyObjectPtr, value: PyObjectPtr
) -> c_int:
var r = self.lib.get_function[
fn (PyObjectPtr, PyObjectPtr, PyObjectPtr) -> c_int
]("PyObject_SetItem")(obj, key, value)

self.log(
"PyObject_SetItem result:",
r,
", key:",
key._get_ptr_as_int(),
", value:",
value._get_ptr_as_int(),
", parent obj:",
obj._get_ptr_as_int(),
)

return r

fn PyObject_GetAttrString(
inout self,
obj: PyObjectPtr,
Expand Down
37 changes: 19 additions & 18 deletions stdlib/src/python/python_object.mojo
Original file line number Diff line number Diff line change
Expand Up @@ -724,29 +724,30 @@ struct PythonObject(
args: The key or keys to set on this object.
value: The value to set.
"""
var cpython = _get_global_python_itf().cpython()
var size = len(args)
var key_obj: PyObjectPtr

var cpython = _get_global_python_itf().cpython()
var tuple_obj = cpython.PyTuple_New(size + 1)
for i in range(size):
var arg_value = args[i].py_object
cpython.Py_IncRef(arg_value)
var result = cpython.PyTuple_SetItem(tuple_obj, i, arg_value)
if result != 0:
raise Error("internal error: PyTuple_SetItem failed")
if size == 1:
key_obj = args[0].py_object
else:
key_obj = cpython.PyTuple_New(size)
for i in range(size):
var arg_value = args[i].py_object
cpython.Py_IncRef(arg_value)
var result = cpython.PyTuple_SetItem(key_obj, i, arg_value)
if result != 0:
raise Error("internal error: PyTuple_SetItem failed")

cpython.Py_IncRef(key_obj)
cpython.Py_IncRef(value.py_object)
var result2 = cpython.PyTuple_SetItem(tuple_obj, size, value.py_object)
if result2 != 0:
raise Error("internal error: PyTuple_SetItem failed")

var callable_obj = cpython.PyObject_GetAttrString(
self.py_object, "__setitem__"
var result = cpython.PyObject_SetItem(
self.py_object, key_obj, value.py_object
)
var result = cpython.PyObject_CallObject(callable_obj, tuple_obj)
cpython.Py_DecRef(callable_obj)
cpython.Py_DecRef(tuple_obj)
Python.throw_python_exception_if_error_state(cpython)
if result != 0:
Python.throw_python_exception_if_error_state(cpython)
cpython.Py_DecRef(key_obj)
cpython.Py_DecRef(value.py_object)

fn _call_zero_arg_method(
self, method_name: StringRef
Expand Down
33 changes: 33 additions & 0 deletions stdlib/test/python/test_python_object.mojo
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,38 @@ fn test_getitem_raises() raises:
_ = with_2d[2]


def test_setitem_raises():
t = Python.evaluate("(1,2,3)")
with assert_raises(
contains="'tuple' object does not support item assignment"
):
t[0] = 0

lst = Python.evaluate("[1, 2, 3]")
with assert_raises(contains="list assignment index out of range"):
lst[10] = 4

s = Python.evaluate('"hello"')
with assert_raises(
contains="'str' object does not support item assignment"
):
s[3] = "xy"

custom = Python.evaluate(
"""type('Custom', (), {
'__init__': lambda self: None,
})()"""
)
with assert_raises(
contains="'Custom' object does not support item assignment"
):
custom[0] = 0

d = Python.evaluate("{}")
with assert_raises(contains="unhashable type: 'list'"):
d[[1, 2, 3]] = 5


def main():
# initializing Python instance calls init_python
var python = Python()
Expand All @@ -500,3 +532,4 @@ def main():
test_none()
test_nested_object()
test_getitem_raises()
test_setitem_raises()

0 comments on commit 64ef6d1

Please sign in to comment.