gemini-code-assist[bot] commented on code in PR #364: URL: https://github.com/apache/tvm-ffi/pull/364#discussion_r2646254300
########## python/tvm_ffi/cython/type_info.pxi: ########## @@ -21,14 +21,15 @@ from io import StringIO cdef class FieldGetter: + __slots__ = () Review Comment:  Adding `__slots__ = ()` is a great step for memory optimization. However, the `cdef dict __dict__` declaration on the next line seems to counteract this by explicitly adding a `__dict__` to each instance. Since this `__dict__` doesn't appear to be used within the class, consider removing it to fully gain the memory benefits of using `__slots__`. ########## python/tvm_ffi/cython/function.pxi: ########## @@ -883,6 +883,7 @@ cdef class Function(Object): :py:func:`tvm_ffi.get_global_func` Look up a previously registered global FFI function by name. """ + __slots__ = () Review Comment:  Adding `__slots__ = ()` is beneficial for memory usage. However, the `cdef dict __dict__` declaration on line 888 seems to contradict this goal by adding a `__dict__` to each `Function` instance. As this `__dict__` is not used, removing it would complete the memory optimization for this class. ########## python/tvm_ffi/cython/type_info.pxi: ########## @@ -37,13 +38,14 @@ cdef class FieldGetter: cdef class FieldSetter: + __slots__ = () Review Comment:  Similar to `FieldGetter`, adding `__slots__ = ()` is good, but the `cdef dict __dict__` on the next line re-introduces a dictionary for each instance. To achieve the intended memory savings, this unused `__dict__` declaration should probably be removed. -- 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]
