Hi Willy Everything in your patch in order to augment code isolation is fine (we just checked it locally), thank you ! >From our point of view, you can proceed to merge it in 1.7-dev
Some words about your performance/accuracy benchmarks: - Performance mode speeds up detection only when working on a mix of desktop web browsers and devices traffic user agents (since it is basically a faster matcher for desktop web browsers that does not attempt the match the real browser). In your test case user agent is never recognized as a web browser so "accuracy mode" performs better than "performance mode" Regards -Scientiamobile On Tue, Nov 8, 2016 at 2:56 PM, Willy Tarreau <[email protected]> wrote: > Hi, > > it's now merged. I've reformated a bit the configuration.txt changes to > fit within the 80-column limit. > > I noticed that it is possible to perform a small change to completely > avoid including wurfl/wurfl.h from the other C files, and which shows > that in the end the wurfl.h file doesn't even need to remain in import/ > since it only declares the new functions. The patch simply consists in > leaving a void * in global.h, allowing to move all the declarations to > wurfl.c so that they are completely isolated : > > diff --git a/include/import/wurfl.h b/include/import/wurfl.h > index c59da34..b3b9335 100644 > --- a/include/import/wurfl.h > +++ b/include/import/wurfl.h > @@ -1,25 +1,7 @@ > #ifndef _IMPORT_WURFL_H > #define _IMPORT_WURFL_H > > -#include <wurfl/wurfl.h> > - > int ha_wurfl_init(void); > void ha_wurfl_deinit(void); > > -typedef char *(*PROP_CALLBACK_FUNC)(wurfl_handle wHandle, > wurfl_device_handle dHandle); > - > -enum wurfl_data_type { > - HA_WURFL_DATA_TYPE_UNKNOWN = 0, > - HA_WURFL_DATA_TYPE_CAP = 100, > - HA_WURFL_DATA_TYPE_VCAP = 200, > - HA_WURFL_DATA_TYPE_PROPERTY = 300 > -}; > - > -typedef struct { > - char *name; > - enum wurfl_data_type type; > - PROP_CALLBACK_FUNC func_callback; > - struct ebmb_node nd; > -} wurfl_data_t; > - > #endif > diff --git a/include/types/global.h b/include/types/global.h > index 1e8caba..f43f4a4 100644 > --- a/include/types/global.h > +++ b/include/types/global.h > @@ -222,7 +222,7 @@ struct global { > struct list patch_file_list; /* the list of WURFL patch > file to use */ > char information_list_separator; /* the separator used in > request to separate values */ > struct list information_list; /* the list of WURFL data to > return into request */ > - wurfl_handle handle; /* the handle to WURFL engine */ > + void *handle; /* the handle to WURFL engine */ > struct eb_root btree; /* btree containing info (name/type) > on WURFL data to return */ > } wurfl; > #endif > diff --git a/src/wurfl.c b/src/wurfl.c > index 0f5ce2d..c869413 100644 > --- a/src/wurfl.c > +++ b/src/wurfl.c > @@ -10,6 +10,8 @@ > #include <proto/sample.h> > #include <ebsttree.h> > #include <ebmbtree.h> > + > +#include <wurfl/wurfl.h> > #include <import/wurfl.h> > > > @@ -32,6 +34,22 @@ inline static void ha_wurfl_log(char * message, ...) > > #define HA_WURFL_MAX_HEADER_LENGTH 1024 > > +typedef char *(*PROP_CALLBACK_FUNC)(wurfl_handle wHandle, > wurfl_device_handle dHandle); > + > +enum wurfl_data_type { > + HA_WURFL_DATA_TYPE_UNKNOWN = 0, > + HA_WURFL_DATA_TYPE_CAP = 100, > + HA_WURFL_DATA_TYPE_VCAP = 200, > + HA_WURFL_DATA_TYPE_PROPERTY = 300 > +}; > + > +typedef struct { > + char *name; > + enum wurfl_data_type type; > + PROP_CALLBACK_FUNC func_callback; > + struct ebmb_node nd; > +} wurfl_data_t; > + > static const char HA_WURFL_MODULE_VERSION[] = "1.0"; > static const char HA_WURFL_ISDEVROOT_FALSE[] = "FALSE"; > static const char HA_WURFL_ISDEVROOT_TRUE[] = "TRUE"; > > > So if you agree, I'll do it as well (and I think we can do the same for > the two other DD engines). > > I could run a quick test on my laptop with your eval version and the > config that is provided as an example (except that I'm doing a redirect > instead of connecting to a server). The client generates a user agent > consisting in "Mozilla/<x>.0" where "x" is an increasing counter. The > client doesn't support keep-alive so it's exlusively in close mode. > > Here are my results : > > perf mode accuracy mode > > no DD: 76000 req/s > no cache: 27500 req/s 29000 req/s > default: 66000 req/s 69000 req/s > simpleLRU(100k) 68000 req/s 69400 req/s > doubleLRU(100k/30k) 68000 req/s 70000 req/s > > The results look good eventhough the UA is fake here. It's surprizing that > the accuracy mode is faster here but maybe it uses a different algorithm > that ends up being faster for such a fake user-agent. You may want to > double-check. > > Please let me know if you're OK with the small change above to isolate > better the code from the rest of the code, the more naming conflicts we > can avoid, the better. > > Cheers, > Willy >
