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
>>>
>
