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