Shrink binary size #168080
Shrink binary size #168080
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/168080
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 5a562a7 with merge base a4e0720 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
@colesbury has exported this pull request. If you are a Meta employee, you can view the originating Diff in D87308588. |
|
@colesbury has imported this pull request. If you are a Meta employee, you can view this in D87308588. |
albanD
left a comment
albanD
left a comment
There was a problem hiding this comment.
Sounds good if we can't measure impact on practical workloads!
| if (C10_UNLIKELY( | ||
| detail::has_pyobject(combined) && | ||
| detail::refcount(combined) == 2)) { | ||
| if (detail::has_pyobject(combined) && detail::refcount(combined) == 2) { |
There was a problem hiding this comment.
In increases the binary size without a corresponding improvement in performance. There wasn't really a good reason to add it in the first place.
|
If we really care about binary size, way faster win would just be enabling Link Time Optimization on the binaries. I have an open issue for this, last got blocked by IntelOneLib/DnnLib having issues with really large statically linked library on MSVC and old clang compiler issues (the later should be fixed now?) |
colesbury
added
topic: not user facing
Summary: Shrink binary size to reduce relocation overflows. The most important change is to split `intrusive_ptr::reset_()` into two functions and mark the bigger one as `C10_NOINLINE`. Reviewed By: albanD Differential Revision: D87308588 Pulled By: colesbury
colesbury
force-pushed
the
export-D87308588
branch
from
fc38adf to
5a562a7
Compare
|
Ugh... I messed up the CI by pushing the same commit to another PR, so re-exporting now |
| friend class pybind11::class_; | ||
|
|
||
| void retain_() { | ||
| void retain_() noexcept { |
There was a problem hiding this comment.
it's not actually noexcept in debug build 🤣
There was a problem hiding this comment.
I think that noexcept means that any escaping exceptions aren't recoverable (they trigger std::terminate). I don't think it means that the function never throws an exception.
The other non-trivial functions here (including reset_() and ~intrusive_ptr()) and can also throw exceptions in rare circumstances, such as broken assertions.
|
@pytorchbot merge (Initiating merge automatically since Phabricator Diff has merged) |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
7 participants
Summary: Shrink binary size to reduce relocation overflows. The most important change is to split
intrusive_ptr::reset_()into two functions and mark the bigger one asC10_NOINLINE.Differential Revision: D87308588