-
Notifications
You must be signed in to change notification settings - Fork 9
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
1258: add missing fields in 'serialize' methods #1263
Conversation
dc44904
to
cf4519f
Compare
This is based on the experimental reports (after reducing the number of false positives).
remaining cases (marked as skipped):
|
41ed4d1
to
abe2e6a
Compare
PR tests (gcc-7, ubuntu, mpich, trace runtime, LB) Build for b1045be
|
PR tests (gcc-5, ubuntu, mpich) Build for b1045be
|
PR tests (clang-3.9, ubuntu, mpich) Build for b1045be
|
PR tests (gcc-10, ubuntu, openmpi, no LB) Build for b1045be
|
PR tests (clang-5.0, ubuntu, mpich) Build for b1045be
|
PR tests (gcc-8, ubuntu, mpich, address sanitizer) Build for b1045be
|
PR tests (gcc-6, ubuntu, mpich) Build for b1045be
|
PR tests (nvidia cuda 11.0, ubuntu, mpich) Build for b1045be
|
PR tests (gcc-9, ubuntu, mpich, zoltan) Build for b1045be
|
PR tests (intel 19, ubuntu, mpich) Build for b1045be
|
PR tests (clang-8, alpine, mpich) Build for b1045be
|
PR tests (intel 18.03, ubuntu, mpich) Build for b1045be
|
PR tests (nvidia cuda 10.1, ubuntu, mpich) Build for b1045be
|
abe2e6a
to
a719cdd
Compare
b1e27e7
to
72ecf80
Compare
Codecov Report
@@ Coverage Diff @@
## develop #1263 +/- ##
===========================================
+ Coverage 81.64% 81.66% +0.01%
===========================================
Files 733 733
Lines 27912 27919 +7
===========================================
+ Hits 22788 22799 +11
+ Misses 5124 5120 -4
|
72ecf80
to
3d62f92
Compare
active_ = static_cast<CallbackEnum>(val); | ||
s.skip(u_); // only serialize actual content of the union |
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.
On second look, I realize that specifically in the footprinting case, serializing just the active member hypothetically undercounts the size of the callback itself, even as the payload of the callback is likely to be substantially larger.
A rigorous solution would be something like s.addBytes(sizeof(u_) - sizeof(u_.active_member_))
I think - i.e. round up the leftover and padding bytes and add that.
As with everything else, the case of concern is if we happen to have many instances of a type accumulating unexpectedly, and blowing up memory, and that's the one we really want footprinting to illuminate.
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.
👍 for noticing this, fixed it along with NoCB
case
06361d4
to
42dd0e1
Compare
42dd0e1
to
b9f57fe
Compare
Fixes nvcc specific error: "base operand of '->' is not a pointer". Note: both gcc and clang were able to compile previous version.
b9f57fe
to
b1045be
Compare
fixes #1258