Introduce a TensorWrapper protection mechanism that holds an additional strong reference for pylayer#78669
Merged
sneaxiy merged 3 commits intoPaddlePaddle:developfrom Apr 24, 2026
Conversation
|
你的PR提交成功，感谢你对开源项目的贡献! |
adam-xiaoyao
force-pushed
the
pylayer-tensor-wrapper-fix
branch
3 times, most recently
from
359d8fa to
75a9396
Compare
Contributor
Author
|
/re-run all-failed |
2 similar comments
Contributor
Author
|
/re-run all-failed |
Contributor
Author
|
/re-run all-failed |
adam-xiaoyao
force-pushed
the
pylayer-tensor-wrapper-fix
branch
3 times, most recently
from
9110683 to
b91245e
Compare
adam-xiaoyao
force-pushed
the
pylayer-tensor-wrapper-fix
branch
6 times, most recently
from
54030eb to
dca931b
Compare
Contributor
Author
|
/re-run all-failed |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #78669 +/- ##
===========================================
Coverage ? 100.00%
===========================================
Files ? 1
Lines ? 2
Branches ? 0
===========================================
Hits ? 2
Misses ? 0
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
adam-xiaoyao
force-pushed
the
pylayer-tensor-wrapper-fix
branch
from
dca931b to
1246b3f
Compare
Contributor
Author
|
/re-run all-failed |
1 similar comment
Contributor
Author
|
/re-run all-failed |
adam-xiaoyao
changed the title
adam-xiaoyao
changed the title
Contributor
Author
|
/re-run all-failed |
1 similar comment
Contributor
Author
|
/re-run all-failed |
adam-xiaoyao
force-pushed
the
pylayer-tensor-wrapper-fix
branch
3 times, most recently
from
e875578 to
7f6a718
Compare
Contributor
Author
|
/re-run all-failed |
2 similar comments
Contributor
Author
|
/re-run all-failed |
Contributor
Author
|
/re-run all-failed |
Comment on lines
+272
to
+273
| VLOG(6) << "Clearing PyLayer tensor wrappers"; | ||
| SetIsTensorWrappersCleared(true); |
Contributor
There was a problem hiding this comment.
这里不应该SetIsTensorWrappersCleared(true);如果你讲vector clear掉，同时Py_DECREF(self->container);的话，才可以。但不建议这么做。这里可以不动。或者说这里与我们要做的事情无关~
Comment on lines
+770
to
+806
|
|
||
| // If tensor_hold_helper is non-empty, some tensors may have been cleared by | ||
| // _clear_dataptr(). Iterate the top-level container tuple and restore any | ||
| // null impl from the corresponding entry in tensor_hold_helper. | ||
| // tensor_hold_helper is ordered by the DenseTensors found during deep | ||
| // traversal in set_container; for the common case (flat tuple of tensors) | ||
| // the k-th tensor in the tuple maps to tensor_hold_helper[k]. | ||
| if (!self->tensor_hold_helper.empty()) { | ||
| Py_ssize_t size = PyTuple_Size(self->container); | ||
| PyObject* recovered_container = PyTuple_New(size); | ||
| Py_ssize_t holder_idx = 0; | ||
| for (Py_ssize_t i = 0; i < size; ++i) { | ||
| PyObject* item = PyTuple_GetItem(self->container, i); | ||
| if (item && PyCheckTensor(item)) { | ||
| TensorObject* tensor_obj = reinterpret_cast<TensorObject*>(item); | ||
| if (!tensor_obj->tensor.impl() && | ||
| holder_idx < | ||
| static_cast<Py_ssize_t>(self->tensor_hold_helper.size()) && | ||
| self->tensor_hold_helper[holder_idx]) { | ||
| // Tensor was cleared by _clear_dataptr; restore impl from holder. | ||
| paddle::Tensor recovered; | ||
| recovered.set_impl(self->tensor_hold_helper[holder_idx]); | ||
| PyTuple_SET_ITEM( | ||
| recovered_container, i, paddle::pybind::ToPyObject(recovered)); | ||
| ++holder_idx; | ||
| continue; | ||
| } | ||
| ++holder_idx; | ||
| Py_INCREF(item); | ||
| PyTuple_SET_ITEM(recovered_container, i, item); | ||
| } else { | ||
| Py_INCREF(item); | ||
| PyTuple_SET_ITEM(recovered_container, i, item); | ||
| } | ||
| } | ||
| return recovered_container; | ||
| } |
Contributor
There was a problem hiding this comment.
这里没有必要吧？直接return self->container;就可以。
Contributor
Author
There was a problem hiding this comment.
只是存到了tensor_hold_helper里， 在self->container里并没有。 因此这里有必要。
Comment on lines
+431
to
+432
| if ctx.closure_cells: | ||
| _restore_freed_closure_tensors(ctx) |
Contributor
There was a problem hiding this comment.
这里还需要保留吗？_restore_freed_closure_tensors是因为有提前释放的问题，孙东对recompute做的修复。有了以上代码，这里我觉得可以直接删掉。你可以找孙东确认一下。
Contributor
Author
There was a problem hiding this comment.
这里是需要的， 我们在release3.3分支上改了的， 当时孙东有参与的。这里主要处理的是recompute的函数为闭包的情况。
adam-xiaoyao
force-pushed
the
pylayer-tensor-wrapper-fix
branch
3 times, most recently
from
f6f1bcc to
6186972
Compare
Problem: PyLayer's save_for_backward only holds Python-level references, but _clear_dataptr releases the C++ DenseTensor data, causing backward failure in pipeline parallel scenarios. Solution: Add C++ TensorWrapper storage to GradNodePyLayer. Registration is done in pylayer_method_apply after ctx->grad_node is set, because forward() is called at line 408 before grad_node is assigned at line 587 - any registration attempted during forward would silently skip due to the null weak_ptr. Changes: - py_layer_node.h: Add saved_tensor_wrappers_ member - py_layer_node.cc: Implement AddSavedTensorWrapper / ClearTensorWrappers - eager_py_layer.cc: Register TensorWrappers in apply() after grad_node is set; recover cleared tensors in tensor_properties_get_container() - recompute.py: Minor compatibility update Tests: 7 test cases covering core pipeline-parallel scenarios
adam-xiaoyao
force-pushed
the
pylayer-tensor-wrapper-fix
branch
2 times, most recently
from
5d05692 to
2e10cda
Compare
adam-xiaoyao
force-pushed
the
pylayer-tensor-wrapper-fix
branch
from
2e10cda to
5a3c9b3
Compare
Contributor
Author
|
/re-run all-failed |
sneaxiy
approved these changes
Apr 24, 2026
Collaborator
sneaxiy
left a comment
sneaxiy
left a comment
There was a problem hiding this comment.
LGTM for placement new.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Category
Execute Infrastructure
PR Types
Bug fixes
Description
For pylayer, Introduce a TensorWrapper protection mechanism that holds an additional strong reference to the DenseTensor at the C++ layer.
是否引起精度变化
否