Skip to content
This repository has been archived by the owner on Mar 22, 2023. It is now read-only.

FEAT: Support for polymorphism in persistent_ptr<> #374

Closed
seghcder opened this issue Jul 16, 2019 · 7 comments
Closed

FEAT: Support for polymorphism in persistent_ptr<> #374

seghcder opened this issue Jul 16, 2019 · 7 comments
Labels
Type: Feature New feature or request Type: Question Further information is requested

Comments

@seghcder
Copy link

Per https://software.intel.com/en-us/articles/an-introduction-to-persistent-memory-pointers-in-c-plus-plus and other references in the documentation:

They (persistent pointers) don’t point to polymorphic objects in C++. This means pointers for the base class are not type-compatible with pointers of a derived class. This is due to the lack of a vtable implementation that can be stored safely in persistent memory. This limitation may be addressed in future versions of the C++ bindings.

Is this likely to be addressed in the nearish future, and/or any thoughts on short term workarounds? :-)

@szyrom
Copy link

szyrom commented Jul 17, 2019

Hi,

lack of polymorphic types support is not a missing implementation in libpmemobj++, but rather missing standard specification for vtable implementation and position in object's layout. So rather than a question "when" we are facing problem "if and how".

For workarounds: we are struggling similar problems in concurrent hashmap implementation, @igchor and @vinser52 may say something more.

About C++ standard limitations in general - you can check out draft version of our blogpost: https://szyrom.github.io/2019/06/26/cpp-limitations.html

@seghcder
Copy link
Author

seghcder commented Jun 4, 2020

Just wondering if you could share more on how you solved the isues in concurrent hashmap and if they might be relevant for this topic?

Also adding xref to #363 (comment) re another workaround for polymorphism using v<>

@igchor
Copy link
Contributor

igchor commented Jun 4, 2020

In concurrent_hash_map we did not really needed polymorphism so I guess guess there's not much to share on that.

One thing I can say (which we basically discovered during hash_map testing) is about inheritance in general. If you have a base class with some members and create a derived class with some additional members then the derived one will not pass std::is_standard_layout<> (at least in C++11, I'm not sure about the later standards). If you care about that (and we do) then even if polymorphism would work use cases would be quite limited.

@seghcder
Copy link
Author

Regarding standard layout, it seems the main driver is:

Standard layout types are useful for communicating with code written in other programming languages. ref

In our case though, it seems we are using this to try to maintain a consistent pool layout. However, it does put some restrictions on us. Is the implied requirement that you want to maintain a pool layout that's compatible between platforms, compilers and compilations? We've seen that might already be an issue with #776. Can we get different benefits if we're willing to stick with a given platform and compiler?

Notwithstanding documentation, would it be ok to use inheritance if the base class contained no member data, only functions? This would allow enforcement of implementation of required virtual functions for example. And since there's no base class member data, the data segment should be concrete. We do have an issue still with vtable, but will come to that...

I used nucleus to try this out. Here is the lab branch. Specific parts:

  • AppBase is a virtual base class for MyApp. It has two pure virtual functions and one with an implementation (start()). This is to see if there's any issues calling it from a derived class.
  • MyApp is derived from AppBase and the functions are overridden.
  • In MyApp::Start() I call the AppBase function.

This all compiles and runs seemingly without issue.

[2020-06-11 08:48:59.244] [main] [debug] AppBase::Start called

Some observations & questions:

  • Since we are using vtable and there's no standard for that, it might change between compilers. What else might change vtable layout (assuming we stay with the same compiler). Even between compiles?
  • Are there any other foreseeable issues with this approach wrt libpmemobj++?
  • Would it be possible to safely cast persistent pointers between objects derived from AppBase? What might this look like?
  • Could these derived objects be placed in a persistent container like pmem::obj::vector?

And some more general questions:

  • How is the text segment of a persistent class managed/stored in Pmem? I found this page quite helpful. But where does the text segment on pmem end and the code of the executable (in DRAM) start?
  • Does this also imply that changing an object implementation also changes the text segment, which could invalidate the pmem layout?

Out of interest, I tried out some of the type traits on these objects in code here:

Output:

AppBase SLayout: false
AppBase DConstruct: false
AppBase TCopy: false
AppBase TDestruct: true

MyApp SLayout: false
MyApp DConstruct: true
MyApp TCopy: false
MyApp TDestruct: false

pmem::obj::string SLayout: true
pmem::obj::string DConstruct: true
pmem::obj::string TCopy: false
pmem::obj::string TDestruct: false

At first I was concerned MyApp didn’t meet the requirements, however it seems pmem::obj::string doesn’t meet requirements for copying and destruction, yet we use it often in vectors etc? It also seems there are no checks for these at present?

Lastly, there are more options I'm still looking into such as static inheritance through templates and separating code from storage like I discuss in #363 (ref)

@pbalcer @szyrom FYI

@igchor
Copy link
Contributor

igchor commented Jun 16, 2020

I don't think there is any compiler which would actually reorder the members (this is what standard layout basically forbids). This is just one step closer to a properly defined behavior. I know there are still things like object lifetime, trivial copying etc. which result in UB but this is just some kind of compromise.

I don't know what specifically can be changed but the problem is that we have no way of rebuilding it reliably. If you would like to do that you would probably have save vtable on pmem (in some fixed format?) and then modify _vptr pointer on each application run (and restoration of _vptr could be compiler specific).

There is no problem with derived classes itself (without virtual methods), you can cast using just static_cast.

The only persistent region is the heap (that's what pmemobj exposes to you). Changing implementation is OK.

@pbalcer
Copy link
Member

pbalcer commented Jun 16, 2020

The very first prototype implementation of C++ bindings actually had polymorphism support:
https://github.com/pbalcer/nvml/blob/obj_cpp_bindings/src/include/libpmemobj.hpp
https://github.com/pbalcer/nvml/blob/obj_cpp_bindings/src/examples/libpmemobj/cpp/example.cpp
(see PMEM_REGISTER_TYPE macro)

But we've decided that it was too fragile and non-portable and so this code was ultimately removed.

@lukaszstolarczuk lukaszstolarczuk added the Type: Feature New feature or request label Jul 7, 2020
@karczex karczex added the Type: Question Further information is requested label Jul 9, 2020
@seghcder
Copy link
Author

seghcder commented Apr 6, 2022

Closing this issue/question, but one interesting observation is that static polymorphism like CRTP https://www.fluentcpp.com/2017/05/12/curiously-recurring-template-pattern/ does seem to work ok :-)

@seghcder seghcder closed this as completed Apr 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Feature New feature or request Type: Question Further information is requested
Projects
None yet
Development

No branches or pull requests

6 participants