tqchen commented on code in PR #327:
URL: https://github.com/apache/tvm-ffi/pull/327#discussion_r2611039975


##########
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 deliberate and not a 
work around, so we do not need to say it is a "hack"



-- 
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]

Reply via email to