Fix serialization to build in both mapped and un-mapped mode#1268
Fix serialization to build in both mapped and un-mapped mode#1268leekillough wants to merge 5 commits intosstsimulator:develfrom
Conversation
…kes it too
complicated, and caused compilation errors. It would be better to store mapped
information in the serializer class state than to pass mapped "name" arguments
at every level of the call hierarchy and to need to split the serialization
functions into mapped and unmapped versions.
Add ser.setMapName() and ser.getMapName() to set and get mapped names, which
are only used in certain places (right now it's just a const char* pointer,
but it can be turned into a std::string if necessary). The SST_SER() macro
will call sst_map_object() function, which will set the quoted variable name
used to be used in the map before the serialization call, and clear the name
after the serialization call, so that the name does not need to be passed
around in every call.
operator() functions which used to be duplicated with a mapped and an unmapped
version, the mapped of which was only valid in serializer::MAP mode, are now
combined into one where the serializer::MAP mode, alread handled in a switch
statement, expects getMapName() to return the name.
Rewrite the default serialize_impl<> class to use modern C++ techniques like
"if constexpr" and "decltype(auto)". tPtr is a new const reference variable
which refers to either t or &t depending on whether t is a pointer or not, so
that pointer and non-pointer code can be merged into one, by using the pointer
version of the code, and it also uses the new t when it is a pointer which
gets allocated in UNPACK mode. The dependent_false template is not needed
anymore, since we can simply use a single static_assert() at the top of our
function to assert that the type, with any top-level pointer removed, is a
non-polymorphic class type.
Write a generalized SST::Core:to_string() which operates like std::to_string
and also handles enumeration types, and generalize SST::Core::from_string()
to handle enumeration types. Right now, enumeration types are inputted and
outputted as integers of the underlying type. There were build errors when
the mapped and non-mapped serialization codes were merged, because the mapped
code depended on std::to_string which isn't defined for enumeration types but
which was depended on when the serialize_impl for "fundamental and enum types"
created a mapping object.
github-actions
bot
added
AT: WIP
|
CLANG-FORMAT TEST - FAILED (on last commit): |
github-actions
bot
added
AT: CMAKE-FORMAT PASS
and removed
AT: WIP
|
CMAKE-FORMAT TEST - PASSED |
|
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
github-actions
bot
added
the
AT: WIP
|
CLANG-FORMAT TEST - FAILED (on last commit): |
github-actions
bot
removed
the
AT: WIP
|
CMAKE-FORMAT TEST - PASSED |
|
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
|
Thanks for putting this together. Minus a few details, this looks very similar to how I first implemented things (including holding name state in the serializer) and was my preferred solution. However, some constraints in the interactions between using serialization for checkpointing and mapping mode (where performance requirements are low) and using the same serialization for event synchronization (where performance requirements are high) led me to the two function solution. I’m on vacation for spring break this week and saw this as I was triaging emails and wanted to give a heads up so you don’t spend too much time on it before we can talk about it. I was under a time constraint in the original implementation and gave up on the one function solution, but maybe we can work something out that gets us back there while fulfilling all the constraints (which haven’t been fully documented yet since mapping mode is still experimental). |
github-actions
bot
added
AT: WIP
|
CMAKE-FORMAT TEST - PASSED |
github-actions
bot
added
AT: WIP
|
CLANG-FORMAT TEST - PASSED |
|
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
berquist
added
the
AT: WIP
4 participants
Fixes tactcomplabs/sst-tools#5 (cross-repo).
Remove the
nameparameter from serializationoperator()(T&, serializer& ser, const char* name), since it makes it too complicated, and caused compilation errors. It would be better to store mapped information in theserializerclass state than to pass mappednamearguments at every level of the call hierarchy and to need to split the serialization functions into mapped and non-mapped versions.Add
ser.setMapName()andser.getMapName()to set and get mapped names, which are only used in certain places(right now it's just a. Theconst char*pointer, but it can be turned into astd::stringif necessary)SST_SER()macro will call thesst_map_object()function, which will set the quoted variable name to be used in the map before the serialization call, and clear the name after the serialization call, so that the name does not need to be passed around in every call.operator()functions which used to be duplicated with a mapped and a non-mapped version, the mapped version of which was only valid inserializer::MAPmode, are now combined into one where theserializer::MAPmode, already handled in aswitchstatement, expectsgetMapName()to return the name.Rewrite the default
serialize_impl<>class to use modern C++ techniques likeif constexpranddecltype(auto).tPtris aconstreference variable which refers to eithertor&tdepending on whethertis a pointer or not, so that pointer and non-pointer code can be merged into one, by using thetPtrpointer to make method calls, andtPtralso reflects the new value oftwhentis a pointer which gets assigned to inUNPACKmode. Thedependent_falsetemplate is not needed anymore, since we can simply use a singlestatic_assert()at the top of our function to assert that the type, with any top-level pointer removed, is a non-polymorphic class type.Write a generalized
SST::Core::to_string()which operates likestd::to_stringand also handles enumeration types, and generalizeSST::Core::from_string()to handle enumeration types. Right now, enumeration types are inputted and outputted as integers of the underlying type. There were build errors when the mapped and non-mapped serialization codes were merged, because the mapped code depended onstd::to_stringwhich isn't defined for enumeration types but which was depended on when theserialize_implfor "fundamental andenumtypes" created a mapping object.For array serialization templates, use
size_tinstead ofintfor the size of the array.The Core tests pass, and Elements builds, but there are some Elements test failures which need investigating and fixing.