gemini-code-assist[bot] commented on code in PR #642:
URL: https://github.com/apache/tvm-ffi/pull/642#discussion_r3460387771
##########
python/tvm_ffi/cython/string.pxi:
##########
@@ -42,6 +42,9 @@ class String(str, PyNativeObject):
val._tvm_ffi_cached_object = None
return val
+ def __reduce_ex__(self, protocol):
+ return (type(self), (str(self),))
Review Comment:
While adding `__reduce_ex__` to `String` correctly prevents pickling the
cached FFI object, the `Bytes` class (which also inherits from `PyNativeObject`
and can carry a cached FFI object) suffers from the same issue. To ensure
consistency and avoid serializing the cached FFI object for `Bytes`, we should
also:\n1. Add `__slots__ = [\"_tvm_ffi_cached_object\"]` to `Bytes` (matching
its declaration in `core.pyi`).\n2. Implement `__reduce_ex__` for `Bytes`
returning `(type(self), (bytes(self),))`.\n\nThis will prevent unnecessary
serialization overhead and potential failures when pickling `Bytes` objects
returned from FFI calls.
##########
tests/python/test_string.py:
##########
@@ -34,6 +35,15 @@ def test_string() -> None:
s4 = pickle.loads(pickle.dumps(s))
assert s4 == "hello"
+ cached = _to_py_class_value(TypeSchema("str").convert("x" * 200))
+ assert isinstance(cached, tvm_ffi.core.String)
+ assert cached._tvm_ffi_cached_object is not None
+
+ cached_roundtrip = pickle.loads(pickle.dumps(cached))
+ assert isinstance(cached_roundtrip, tvm_ffi.core.String)
+ assert cached_roundtrip == cached
+ assert cached_roundtrip._tvm_ffi_cached_object is None
Review Comment:
It would be highly beneficial to also add a test case for pickling `Bytes`
with a cached FFI object, ensuring that its cached object is also dropped upon
unpickling (once the corresponding `__reduce_ex__` is implemented for `Bytes`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
