On Thu, Jun 1, 2017 at 10:22 PM, Yann Ylavic <ylavic....@gmail.com> wrote: > On Thu, Jun 1, 2017 at 7:29 PM, Jim Riggs <apache-li...@riggs.me> wrote: >>> On 1 Jun 2017, at 07:55, Jim Jagielski <j...@jagunet.com> 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?
... a simple 'best' *flag* (as argument) ... > > 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.
