tqchen commented on code in PR #327:
URL: https://github.com/apache/tvm-ffi/pull/327#discussion_r2611041886
##########
python/tvm_ffi/error.py:
##########
@@ -121,19 +121,29 @@ def append_traceback(
The new traceback with the appended frame.
"""
- frame = self._create_frame(filename, lineno, func)
- return types.TracebackType(tb, frame, frame.f_lasti, lineno)
+
+ # Magic hack to prevent a circular reference
+ def create(
+ tb: types.TracebackType | None, frame: types.FrameType, lineno: int
+ ) -> types.TracebackType:
+ return types.TracebackType(tb, frame, frame.f_lasti, lineno)
+
+ return create(tb, self._create_frame(filename, lineno, func), lineno)
_TRACEBACK_MANAGER = TracebackManager()
def _with_append_backtrace(py_error: BaseException, backtrace: str) ->
BaseException:
"""Append the backtrace to the py_error and return it."""
- tb = py_error.__traceback__
- for filename, lineno, func in _parse_backtrace(backtrace):
- tb = _TRACEBACK_MANAGER.append_traceback(tb, filename, lineno, func)
- return py_error.with_traceback(tb)
+ # we manually delete py_error and tb to avoid reference cycle, making it
faster to gc the locals inside the frame
+ try:
+ tb = py_error.__traceback__
+ for filename, lineno, func in _parse_backtrace(backtrace):
+ tb = _TRACEBACK_MANAGER.append_traceback(tb, filename, lineno,
func)
+ return py_error.with_traceback(tb)
+ finally:
+ del py_error, tb
Review Comment:
@KEKE046 shall we apply genimi suggestion here?
##########
python/tvm_ffi/error.py:
##########
@@ -121,19 +121,65 @@ def append_traceback(
The new traceback with the appended frame.
"""
- frame = self._create_frame(filename, lineno, func)
- return types.TracebackType(tb, frame, frame.f_lasti, lineno)
+
+ # The `create` function is a hack to prevent append_traceback from
holding its child frame
+ # please see the reference cycle diagram in _with_append_backtrace and
pull request #327 for more details
+ #
+ # This hack prevent binding `self._create_frame` to a writable local
variable
Review Comment:
This hack => This approach avoid binding `self._create_frame` to a writable
local variable
##########
python/tvm_ffi/error.py:
##########
@@ -121,19 +121,65 @@ def append_traceback(
The new traceback with the appended frame.
"""
- frame = self._create_frame(filename, lineno, func)
- return types.TracebackType(tb, frame, frame.f_lasti, lineno)
+
+ # The `create` function is a hack to prevent append_traceback from
holding its child frame
+ # please see the reference cycle diagram in _with_append_backtrace and
pull request #327 for more details
+ #
+ # This hack prevent binding `self._create_frame` to a writable local
variable
+ # Thus, frame object is a temporary object that won't be held by the
locals of append_traceback
+ def create(
+ tb: types.TracebackType | None, frame: types.FrameType, lineno: int
+ ) -> types.TracebackType:
+ return types.TracebackType(tb, frame, frame.f_lasti, lineno)
+
+ return create(tb, self._create_frame(filename, lineno, func), lineno)
_TRACEBACK_MANAGER = TracebackManager()
def _with_append_backtrace(py_error: BaseException, backtrace: str) ->
BaseException:
"""Append the backtrace to the py_error and return it."""
- tb = py_error.__traceback__
- for filename, lineno, func in _parse_backtrace(backtrace):
- tb = _TRACEBACK_MANAGER.append_traceback(tb, filename, lineno, func)
- return py_error.with_traceback(tb)
+ # We manually delete py_error and tb to avoid reference cycle, making it
faster to gc the locals inside the frame
+ # please see pull request #327 for more details
+ #
+ # Memory Cycle Diagram:
+ #
+ # [Stack Frames] [Heap Objects]
+ # +-------------------+
+ # | outside functions | -----------------------> [ Tensor ]
+ # +-------------------+ (Held by cycle, slow to free)
+ # ^
+ # | f_back
+ # +-------------------+ locals py_error
+ # | py_error (this) | -----+--------------> [ BaseException ]
+ # +-------------------+ | |
+ # ^ | | (with_traceback)
+ # | f_back | v
+ # +-------------------+ +--------------> [ Traceback Obj ]
+ # | append_traceback | tb |
+ # +-------------------+ |
+ # ^ |
+ # | f_back |
+ # +-------------------+ |
+ # | _create_frame | |
+ # +-------------------+ |
+ # ^ |
+ # | f_back |
+ # +-------------------+ |
+ # | _get_frame | <----------------------------+
+ # +-------------------+ (Cycle closes here)
+ try:
+ tb = py_error.__traceback__
+ for filename, lineno, func in _parse_backtrace(backtrace):
+ tb = _TRACEBACK_MANAGER.append_traceback(tb, filename, lineno,
func)
+ return py_error.with_traceback(tb)
+ finally:
+ # this is a hack to break reference cycle
Review Comment:
we explicitly break reference cycle here
--
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]
