geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Kimura <dkim...@pivotal.io>
Subject Re: Return types form methods on Cache
Date Tue, 19 Sep 2017 18:22:45 GMT
Cool. +1 value model.

Thanks,
David

On Tue, Sep 19, 2017 at 11:15 AM, Jacob Barrett <jbarrett@pivotal.io> wrote:

> It isn’t. A Region should not outlive cache.
>
> > On Sep 19, 2017, at 10:54 AM, David Kimura <dkimura@pivotal.io> wrote:
> >
> > Oh, man. I hope I didn't just kill the original idea.  Unless we are
> > somehow using shared_from_this, it shouldn't be a divergence from
> existing
> > behavior?
> >
> > Thanks,
> > David
> >
> >> On Tue, Sep 19, 2017 at 10:47 AM, Jacob Barrett <jbarrett@pivotal.io>
> wrote:
> >>
> >> Region returned by this would no longer be valid. It’s “references” to
> >> interns cache/region would be invalid. The behavior is undefined.
> >>
> >>> On Sep 19, 2017, at 10:24 AM, David Kimura <dkimura@pivotal.io> wrote:
> >>>
> >>> Err... I missed a return in example above.
> >>>
> >>> Region r = [](){ return CacheFactory::create().getRegion("myregion");
> }
> >> ();
> >>>
> >>>> On Tue, Sep 19, 2017 at 10:19 AM, David Kimura <dkimura@pivotal.io>
> >> wrote:
> >>>>
> >>>> I favor values, but we should probably be diligent.
> >>>>
> >>>> Do any of the objects returned from Cache depend on Cache to out-live
> >> the
> >>>> returned object?  A quick scan suggested no, but we still have a
> >>>> std::enable_shared_from_this<Cache>.  Maybe dead code.  In code
> >> example,
> >>>> if this happens (for any cache.get*), could we be in trouble or is
> this
> >>>> user error?
> >>>>
> >>>> Region r = [](){ CacheFactory::create().getRegion("myregion"); }();
> >>>> // Cache is out of scope.
> >>>> // What is expected behavior?
> >>>> r.put("key", "value");
> >>>>
> >>>> Thanks,
> >>>> David
> >>>>
> >>>> On Mon, Sep 18, 2017 at 7:44 PM, Jacob Barrett <jbarrett@pivotal.io>
> >>>> wrote:
> >>>>
> >>>>>
> >>>>>
> >>>>>> On Sep 18, 2017, at 7:34 PM, Kirk Lund <klund@apache.org>
wrote:
> >>>>>>
> >>>>>> I would vote +1 for a more attractive, professional and
> user-friendly
> >>>>> API.
> >>>>>> I'm not sure if there's a perf or memory-usage reason for using
> >>>>>> "std::shared_ptr<?>" to types instead of returning values,
but the
> end
> >>>>>> result does not look like a professional API to me.
> >>>>>
> >>>>> There really isn’t, especially if you look at what we did dirty
> >>>>> CacheFactory::getCache by returning a value that can be moved into
> the
> >> heap
> >>>>> and a shared point of one wants but not being forced into it. RVO
> >> tricks
> >>>>> can event make that move a noop.
> >>>>>
> >>>>> auto r = cache.getRegion(...);
> >>>>> Where decltype(r) == Region
> >>>>> and
> >>>>> auto rp = std::make_shared<Region>(cache->getRegion());
> >>>>> Where decltype(rp) == shared_ptr<Region>
> >>>>>
> >>>>> Would both be valid.
> >>>>>
> >>>>>
> >>>>>
> >>>>
> >>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message