On Thu, Jun 1, 2017 at 7:29 PM, Jim Riggs <[email protected]> wrote: >> On 1 Jun 2017, at 07:55, Jim Jagielski <[email protected]> wrote: >> >> I really like where this is going... I just have a few questions: >> >> 1. The style, esp with array usage is different; eg APR_ARRAY_PUSH >> and APR_ARRAY_IDX... any particular reason why? > > Well, we don't seem to be entirely consistent with using elts and > apr_array*() directly vs. the helper #defines across the codebase. I > was doing everything with direct pointer access and arithmetic until > I started looking at the heartbeat lbmethod that was using some of > the helpers. IMNSHO, I think they are a little cleaner and easier > use/read. I got lost in dereferencing hell with all of the `&((*...) > **)->(*)'! :-) The helpers are there, and I think they are convenient > and worth using, but I'm not digging through all of this code as > regularly as some of the others on here. If the standard/consensus is > the other way, it's easy to change.
I also (much, much) prefer the macros, and preferably would like all the 'elts' usages and cast trickeries changed to them rather. But that's another (own) story... > > >> 2. I understand the logic behind creating the arrays, but doesn't >> this increase the overhead. We go thru the full list of workers >> one time, and then go thru the list of avail works and spares >> right after that. Assume that all workers are available; doesn't >> it mean we go thru that last 2x? [] > > The only way I can think of to avoid this without going back to > duplicating code across lbmethods, which I would argue is worse, is > to maybe have the lbmethod provide a callback function and context > pointer to ap_proxy_balancer_usable_workers() that gets called during > the loop iteration to pick the best member in flight. Couldn't a simple 'best' for ap_proxy_balancer_usable_workers() make it return a single entry? By the way, retrieving ap_proxy_retry_worker() via an OPTIONAL_FN looks unnecessary since it's implemented in the exact same "proxy_util.c" file. > > Regardless, even worst case, we are looking at what, iterating 6 > pointers instead of 3 or 10 instead of 5? We probably have some lower > hanging fruit across the request lifecycle code to increase > performance than saving some arithmetic on a handful of structs, no? > ;-) Agreed.
