On Friday 05 February 2010, Dan Poirier wrote:
> On Wed, Feb 3, 2010, at 03:22:21 AM, Stefan Fritsch
<[email protected]> 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
