-
Notifications
You must be signed in to change notification settings - Fork 5.7k
[SOT][FasterGuard] add ExprNodeBase
cleanup func
#72552
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
[SOT][FasterGuard] add ExprNodeBase
cleanup func
#72552
Conversation
你的PR提交成功,感谢你对开源项目的贡献! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a cleanup mechanism within expression nodes to ensure temporary Python objects are properly released, thereby improving performance.
- Introduces a virtual cleanup() method in ExprNodeBase and corresponding overrides in AttributeExprNode and ItemExprNode.
- Implements a macro (DELAYED_CLEAN) to decrement reference counts and clean up stored Python objects.
- Updates evaluation functions to record cleanupable objects during attribute and item access.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
paddle/fluid/pybind/sot/guards.h | Added cleanup() methods and storage vectors in expression nodes. |
paddle/fluid/pybind/sot/guards.cc | Introduced DELAYED_CLEAN macro and updated eval methods. |
Comments suppressed due to low confidence (1)
paddle/fluid/pybind/sot/guards.h:354
- [nitpick] Consider renaming 'clean_py_obj_' to 'cleanup_py_objects_' to improve clarity regarding its purpose.
std::vector<PyObject*> clean_py_obj_;
paddle/fluid/pybind/sot/guards.cc
Outdated
bool should_erase = true; \ | ||
if (*it) { \ | ||
Py_DECREF(*it); \ | ||
should_erase = (Py_REFCNT(*it) <= 0); \ | ||
} \ | ||
it = should_erase ? clean_py_obj_.erase(it) : ++it; \ | ||
} \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling Py_REFCNT on a PyObject immediately after a Py_DECREF may cause undefined behavior if the object is deallocated when its reference count reaches zero. Consider updating the cleanup logic to avoid accessing deallocated memory.
bool should_erase = true; \ | |
if (*it) { \ | |
Py_DECREF(*it); \ | |
should_erase = (Py_REFCNT(*it) <= 0); \ | |
} \ | |
it = should_erase ? clean_py_obj_.erase(it) : ++it; \ | |
} \ | |
if (*it) { \ | |
PyObject* temp = *it; \ | |
Py_DECREF(temp); \ | |
it = clean_py_obj_.erase(it); \ | |
} else { \ | |
++it; \ | |
} \ |
Copilot uses AI. Check for mistakes.
PR Category
Performance Optimization
PR Types
Performance
Description
添加清理方法,用于解决过程中临时变量的清理
TODO: