On Friday 05 February 2010, Dan Poirier wrote: > On Wed, Feb 3, 2010, at 03:22:21 AM, Stefan Fritsch <s...@sfritsch.de> wrote: > > ap_log_error_wrapper.diff: > > On C99 compilers, avoid argument setup and function call overhead > > if the log message will be discarded anyway. Also allow to > > disable higher loglevels at compile time by defining > > APLOG_MAX_LOGLEVEL. On pre-C99 compilers, it should just work > > like before. > > This seems like a reasonable thing to do. I can't comment on the > correctness, not being up on C99.
It works with gcc, both with and without -std=gnu99, and creates different code accordingly. It would be very nice if someone could test it with non-gcc compilers (Netware, Windows?). I just noticed a small bug: It doesn't implement the logic to always log messages with level notice. This is easily fixed by an additional check that will be optimized away in most cases. For example: --- a/include/http_log.h +++ b/include/http_log.h @@ -109,12 +109,12 @@ extern "C" { #ifndef APLOG_MAX_LOGLEVEL #define APLOG_IS_LEVEL(s,level) \ - ((s == NULL) || \ + (((level) >= APLOG_NOTICE) || (s == NULL) || \ ((s)->loglevel >= ((level)& APLOG_LEVELMASK))) #else #define APLOG_IS_LEVEL(s,level) \ (((level) <= APLOG_MAX_LOGLEVEL) && \ - ((s == NULL) || \ + (((level) >= APLOG_NOTICE) || (s == NULL) || \ ((s)->loglevel >= ((level) & APLOG_LEVELMASK)))) #endif > Is there some server coding convention calling for trailing _ and > __ on the macro and function names? If not, my personal > preference would be something more meaningful when reading the > code. I just couldn't think of a really meaningful name. ap_do_log_*error? ap_real_log_*error? > I'd love to know difference this makes in processor usage under > load, between running with loglevel debug and something lower. > Saving a function call for every logging line on the main path > could be a big win. I am not sure it makes noticable difference, except maybe in mod_ssl code which has quite a lot of debug logging. But it allows to insert even more debug logging without sacrificing performance. See below for an example. > > loglevel_trace.diff: > > Introduce additional log levels trace1 ... trace8 above the debug > > level. > > If we're thinking about expanding control of debug messages, my > inclination would be to work toward allowing independent control of > messages from different parts of the code or about different > things, rather than a strict series of increasing levels of > logging. E.g. maybe today I'd like to see all debug trace from > authentication, but tomorrow just see SSL stuff. Absolutely. I want to have per-module log levels, too. Adding more log levels is just a first step and a necessary condition for adding more debug logging and getting rid of things like RewriteLogLevel and LogLevelDebugDump. And I thought that small patches are more likely to get reviewed. Example use case for more log levels in mod_proxy_http: level X: log request line sent to backend server and response code received from backend level X+1: log things like local port used, timing info for establishing connection, sending request, receiving response level X+2: dump all headers sent to backend and all headers received in response from backend level X+3: dump complete request sent to backend level X+4: dump complete request received from backend Obviously this is not possible with only one debug log level. I am currently improving the per-module loglevel patch a bit. I am also looking at per-directory loglevel configuration and separate additional logfiles. Think of the following use cases: RewriteLog/RewriteLogLevel equivalent: Loglevel warn rewrite:warn,logs/rewrite.log=trace4 Logging requests from one client at level debug to debug.log without affecting the normal error log in any way: Loglevel warn <If %{REMOTE_ADDR} = 192.168.1.2 > Loglevel warn,logs/debug.log=debug </If> This will still take some time to get finished, though. Maybe I should create a svn branch for all the log refactoring work? Cheers, Stefan