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