Calling a vote for:
- Raw pointer
- shard pointer
+1 raw Pointer, I had to look up RVO and am new to std::move(s)
On Thu, Sep 14, 2017 at 3:02 PM, Michael William Dodge <mdo...@pivotal.io>
wrote:
> I generally dig reference-counted pointers for avoiding lifetime issues
> with objects allocated off the heap but I can live with bare pointers, too.
>
> Sarge
>
> > On 13 Sep, 2017, at 16:25, Mark Hanson <mhan...@pivotal.io> wrote:
> >
> > Hi All,
> >
> > I favor the “pointer" approach that is identified in the code sample.
> There is greater clarity and less bytes seemingly created and written. We
> do sacrifice the potential ease of using an object, but in all, I think the
> way our code is structured. It is not conducive to do a value approach,
> from an efficiency standpoint, because of our use of the pimpl model.
> >
> > Thanks,
> > Mark
> >
> >> On Sep 12, 2017, at 11:09 AM, Jacob Barrett <jbarr...@pivotal.io>
> wrote:
> >>
> >> My biggest concern with this model is that access to the public Cache
> >> object from other public objects results in additional allocations of a
> >> Cache value. Think about when we are inside a Serializable object and we
> >> access the Cache from DataOutput.
> >>
> >> As value:
> >> Serializable* MyClass::fromData(DataInput& dataInput) {
> >> auto cache = dataOutput.getCache();
> >> ...
> >> }
> >> In this the value of cache will RVO the allocation of Cache in the
> getCache
> >> call into the stack of this method, great. The problem is that Cache
> must
> >> contain a std::shared_ptr<CacheImpl> which means that each allocation
> is 8
> >> bytes (pointer to control block and pointer to CacheImpl) as well as
> having
> >> to increment the strong counter in the control block. On exit/descope,
> the
> >> Cache will have to decrement the control block as well.
> >>
> >> Using current shared_ptr pimple model:
> >> Serializable* MyClass::fromData(DataInput& dataInput) {
> >> auto& cache = dataOutput.getCache();
> >> ...
> >> }
> >> We only suffer the ref allocation of 4 bytes and no ref count. Since
> this
> >> function call can't survive past the lifespan of Cache/CacheImpl they
> don't
> >> need to have shared_ptr and refcounting.
> >>
> >> Given that this method could be called numerous times is the overhead of
> >> the value version going to be a significant performance issue?
> >>
> >> I worry that moves and RVO is just beyond most developers. Heck I didn't
> >> know anything about it until we started exploring it.
> >>
> >>
> >>
> >> -Jake
> >>
> >>
> >> On Tue, Sep 12, 2017 at 8:06 AM David Kimura <dkim...@pivotal.io>
> wrote:
> >>
> >>> Follow up of attached discussion after more investigation. I created
> an
> >>> example of returning Cache as shared pointer versus raw value:
> >>>
> >>> https://github.com/dgkimura/geode-native-sandbox
> >>>
> >>> I still like returning by value as it lets the user do what they want
> with
> >>> their object.
> >>>
> >>> // Here user creates object on their stack.
> >>> auto c = CacheFactory::createFactory().create();
> >>>
> >>> // Here user creates smart pointer in their heap.
> >>> auto cptr =
> >>> std::make_shared<Cache>(CacheFactory::createFactory().create());
> >>>
> >>> Difficulty of implementing this is high due to circular dependencies of
> >>> Cache/CacheImpl as well as objects hanging off CacheImpl that return
> >>> Cache. We must be extra careful when dealing with move/copy semantics
> of
> >>> Cache/CacheImpl.
> >>>
> >>> Alternative, is to keep as is and only permit heap allocations from
> >>> factory using shared pointers.
> >>>
> >>> Thanks,
> >>> David
> >>>
> >
>
>
