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.

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

Reply via email to