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