I was planning on replacing the internals of TxnArg with the Extendible. I
haven't fully fleshed out the API. I had planned to keep the C API backward
compatible. But I really want to dive into a C++ API with type safety,
which was a major goal in the Extendible implementation.
On Fri, Mar 6, 2020 at 9:09 PM Alan Carroll
<solidwallofc...@verizonmedia.com.invalid> wrote:
> I am in favor of this because although the replace of In Name Only types
> with void * makes a more risky API, it is (in my judgement) a relatively
> minor risk especially since almost all of those types are in fact
> Continuations and the internals can
> check the dynamic_cast for success. I think it is more than balanced by the
> cleaner, thinner, and more regular API. Obviously this is a judgement call,
> but my judgement is this is an improvement overall. It has a clear benefit
> and a relatively minor cost.
>
> On Wed, Mar 4, 2020 at 1:29 PM Leif Hedstrom <zw...@apache.org> wrote:
>
> >
> >
> > > On Mar 4, 2020, at 12:18 PM, Walt Karas <wka...@verizonmedia.com
> .INVALID>
> > wrote:
> > >
> > > Instead of int argument indexes, how about argument IDs of type:
> > >
> > > typedef struct
> > > {
> > > TSUserArgType type;
> > > short idx;
> > > } TSUserArgIDType;
> > >
> > > This would allow for more run-time type checking.
> >
> >
> > Not sure I follow that… The type safety stuff is in the Get() / Set()
> > functions void*, not the index, and this doesn’t help that ? Also, seems
> > non-ATS API to require this additional struct .
> >
> > — Leif
> >
> >
> > >
> > > On Wed, Mar 4, 2020 at 12:40 PM Alan Carroll
> > > <solidwallofc...@verizonmedia.com.invalid> wrote:
> > >
> > >> I like thinner and more automatic APIs. This is less typing overall,
> > >> because the Get/Set functions don't require a type name in them. It
> also
> > >> makes it easier to add or remove object args. Also, in the longer term
> > (ATS
> > >> 10) we should move to Extendible anyway, which provides real type
> > safety.
> > >>
> > >> On Wed, Mar 4, 2020 at 12:31 PM Leif Hedstrom <zw...@apache.org>
> wrote:
> > >>
> > >>>
> > >>>
> > >>>> On Mar 4, 2020, at 11:17 AM, Walt Karas <wka...@verizonmedia.com
> > >> .INVALID>
> > >>> wrote:
> > >>>>
> > >>>> So why do you like this?
> > >>>
> > >>> - Less code
> > >>> - Fewer APIs
> > >>> - Generalizes the concept
> > >>>
> > >>>
> > >>> I don’t feel super strongly about the Get() / Set()’ers, I can
> > definitely
> > >>> retain the underlying core changes (which are good), and keep (and
> add
> > >>> more) of the old style APIs). Now, had you guys not -1’d my
> suggestion
> > to
> > >>> turn ts/ts.h into a C++ header file for 9.0.0, we could totally have
> > made
> > >>> these two APIs type safe. ;-)
> > >>>
> > >>> I do feel strongly about the “register” functions though, there’s no
> > >> point
> > >>> in 4x’ing those again, and they are most definitely type safe in this
> > >>> version, and just overall cleaner.
> > >>>
> > >>> I’ll let amc decide how strongly he prefers this over the explicit
> > (more
> > >>> type safe) APIs for the Get()/ Set()’ers.
> > >>>
> > >>> Cheers,
> > >>>
> > >>> — Leif
> > >>>
> > >>>
> > >>>>
> > >>>> - Breaks lots of existing code.
> > >>>> - Less typesafe.
> > >>>> - Makes use more verbose.
> > >>>>
> > >>>> It's good how the API has avoided the use of void pointers. We
> should
> > >>> not
> > >>>> reverse that design goal.
> > >>>>
> > >>>> On Wed, Mar 4, 2020 at 12:03 PM Leif Hedstrom <zw...@apache.org>
> > >> wrote:
> > >>>>
> > >>>>>
> > >>>>>
> > >>>>>> On Feb 27, 2020, at 10:13 AM, Leif Hedstrom <zw...@apache.org>
> > >> wrote:
> > >>>>>>
> > >>>>>> Hi all,
> > >>>>>>
> > >>>>>> This is an idea from Bryan Call, which he suggested as a solution
> > for
> > >>>>> reloadable plugins changes that are now in 9.0.x. I recently ran
> into
> > >> a
> > >>>>> different use case (thanks Miles …), which could really benefit
> > >>>>> (performance wise) with such APIs. I *could* use the TxnArg APIs
> > >>> instead,
> > >>>>> but these new global arg APIs would make it noticeable faster (and
> > >> less
> > >>>>> locking). So, I’m writing up this proposal based on these ideas,
> for
> > >>>>> discussion / adjustments.
> > >>>>>
> > >>>>>
> > >>>>> After discussions here, and with help from Alan and Bryan, I’ve
> made
> > >>> some
> > >>>>> significant changes to these APIs now. Here’s the new design:
> > >>>>>
> > >>>>> 1) All old APIs are marked deprecated!
> > >>>>> 2) 5 new APIs are introduced
> > >>>>> 3) The Global (GLB) type of user args is added.
> > >>>>>
> > >>>>>
> > >>>>> Item #2 now makes the API look like this:
> > >>>>>
> > >>>>> tsapi TSReturnCode TSUserArgIndexReserve(TSUserArgType type, const
> > >> char
> > >>>>> *name, const char *description, int *arg_idx);
> > >>>>> tsapi TSReturnCode TSUserArgIndexNameLookup(TSUserArgType type,
> const
> > >>> char
> > >>>>> *name, int *arg_idx, const char **description);
> > >>>>> tsapi TSReturnCode TSUserArgIndexLookup(TSUserArgType type, int
> > >> arg_idx,
> > >>>>> const char **name, const char **description);
> > >>>>>
> > >>>>> tsapi void TSUserArgSet(void *data, int arg_idx, void *arg);
> > >>>>> tsapi void *TSUserArgGet(void *data, int arg_idx);
> > >>>>>
> > >>>>>
> > >>>>> The first three now takes a TSUserArgType argument:
> > >>>>>
> > >>>>> typedef enum {
> > >>>>> TS_USER_ARGS_TXN, ///< Transaction based.
> > >>>>> TS_USER_ARGS_SSN, ///< Session based
> > >>>>> TS_USER_ARGS_VCONN, ///< VConnection based
> > >>>>> TS_USER_ARGS_GLB, ///< Global based
> > >>>>> TS_USER_ARGS_COUNT ///< Fake enum, # of valid entries.
> > >>>>> } TSUserArgType;
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>> The Get()/Set() are then context sensitive. You pass in a “txnp”,
> it
> > >>> uses
> > >>>>> the TXN slots, you pass in a “ssnp”, it uses the SSN slot. For the
> > >> “GLB”
> > >>>>> slots, simply pass in a nullptr (since the global doesn’t have a
> > >>> context).
> > >>>>>
> > >>>>> Thoughts? One question is if we should change the “int” type on the
> > >>> index
> > >>>>> to e.g. “size_t”. That’s slightly unorthodox for our existing APIs,
> > >>> where
> > >>>>> we frivolously use “int” for such things (so size_t would be
> somewhat
> > >>>>> unorthodox in ts/ts.h).
> > >>>>>
> > >>>>> — Leif
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>
> > >>>
> > >>
> >
> >
>
