On 18-03-2012 21:50, Matthew Toseland wrote:
On Sunday 18 Mar 2012 23:40:44 Marco Schulze wrote:
One thing has been bothering me: those 'if (logMINOR) Logger.minor(...',
and the mess that logging is inside fred. I've written a very simple
replacement for Logger + associated classes with the following changes:
You're not using the java logging API?
First thing I thought, but the Java logging mechanisms are complex (more
LoC), at least compared to what I have now. It also lacks those cute
error()/debug() methods, so a wrapper would be needed anyway.
The if(logMINOR) is *ABSOLUTELY VITAL* for performance. You simply cannot get
rid of it. Generating the strings costs significantly more CPU than the rest of
what Freenet does put together.
In most cases I've seen so far, either the message is constant, or is
interpolated with one or two variables. In those cases where building a
message to be logged is actually expensive, directly checking the logger
state (Logger.min_severity) saves that contrived prolog used to setup
logMINOR and logDEBUG.
If string building is really slow (possibly because some object's
toString() is going wild), logging methods can be changed to something
like: public void Logger.info(Object... objs), and the final string
would only be built by the logger if it would actually use it.
- Log level (renamed to severity) filtering is done by Logging.log();
Feel free to rename it. I don't see much point though unless you are trying to
use a standard API, which *may* be a good idea.
No special reason for the change, I just though Logger.Severity sounded
better than Logger.LogLevel.
- Specific writer classes are replaced by a simple OutputStream, which
defaults to System.err. Formatting is also unified;
Very bad idea. There are many excellent reasons for the complexity that is
FileLoggerHook. One is that if System.err is blocked (e.g. a full disk),
everything grinds to a halt.
Doesn't write() throw an IOException when the disk is full?
Adequate caching is also essential, and I'm not certain whether there are
locking issues;
You mean, caching of log messages? Could you elaborate?
and dropping log lines when we can't keep up rather than just slowing down the
rest of the node is a good thing.
Sorry, but I heavily disagree with you here. Not only dropping messages
makes logs misleading, the output stream should either be unbuffered or
flushed after every message. If everything is slowing down because of
this, it is either expected (DEBUG or TRACE messages are written), or
fred is logging too much above the default INFO threshold.
- Severity cases are broadened (FATAL, ERROR, WARNING, INFO, DEBUG and
TRACE), MINOR is mapped to DEBUG, and NORMAL is mapped to INFO;
Using the standard classes is fine by me.
The biggest problem is that MINOR and NORMAL aren't being used the same
way by all classes, and their use isn't exactly clear.
- No logging method accepts an Object parameter - hashCode() is not
exactly useful.
We're not after the hashcode, we're after the class name. We need to be able to
filter by it, and that is essential.
The problem is disambiguation, or per-object log threshold?
Additionally, log rotation will be moved outside (possibly inside Node).
Not a good idea, for reasons of locking, file handle handling, the fact that
you can't rename a file that is open on Windows ...
I see. A LogRotator would solve it then.
There are GOOD REASONS for all the complexity, but feel free to tinker with the API, or
give specific arguments other than "this is all so complicated it *ought to* be
possible to do something radically simpler".
Not that it *ought to be* simpler, I just think current requirements are
a bit exaggerated.
Currently, the log format is '<severity>\t<message>'.
We need the timestamp, in a great many cases.
In my todo list. Which object keeps track of uptime?
_______________________________________________
Devl mailing list
[email protected]
https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl
_______________________________________________
Devl mailing list
[email protected]
https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl
