Being a devil's advocate here, I'd like to point out that the particular
example given:
> logger.log(level, () -> String.format(...));
is better done through an API like this one:
logger.logf(level, fmt, ...);
which would in fact allow for the possibility of localization (using the
format string as a key as is done for MessageFormat-style messages)
while also allowing the same performance optimizations that are allowed
in other cases (we actually have such API methods on our LogManager's
Logger implementation).
The place where this optimization is generally most useful is in fact in
the argument list - this is where it is most common for expensive values
to be computed.
On 12/27/2012 02:40 PM, Brian Goetz wrote:
I think a significant fraction of the community would disagree with you.
We ran a survey where we collected suggestions for lambdafying API
methods, and this one came in top of the list.
There is a significant fraction of the developer community that uses the
logging API and doesn't care at all about localization, but does care
about logging performance. One doesn't have to look very far to see
that it is common practice to surround logging calls with
if (logger.isLogging(level))
logger.log(level, msgExpr)
to work around the eager evaluation. And such a practice is brittle,
because it's easy to forget to do it in one place, and lose the benefit.
Now, your answer might be "force all users to use message catalogs." But
that's pretty mean. A lot of users really, really don't want to use
message catalogs. They want to do:
logger.log(level, () -> String.format(...));
You're basically saying we should force-feed those users some
message-catalog vegetables, because it's good for them.
Henry,
Please don't apply this patch. This patch and the suggested
workarounds are still an anti-pattern of the logging API. You don't
want to encourage this type of on the fly message construction because
it can't be localized. Even Netbeans has a code hint to undo this
pattern that you are enabling. The problem with this patch is it fails
to meet even its own goals when used it in a real world context. The
stated goal is to eliminate unnecessary message construction but, in
this patch you will pay the cost of message construction when you
create a LogRecord. If you configure a system with MemoryHandler to
track the events that lead up to a failure you will pay the message
cost on every LogRecord that passes through the ring buffer. With
this API change, we are performing costly message construction for
evicted and unformatted records which is awful. This same kind of
worst case behavior happens when the handler levels are higher than
the logger level or if a handler is !
using a filter to track a specific error. I've used combinations of
those logging configurations on production applications to track down
elusive errors (See bug 6219960). This patch assumes that if a record
is loggable by a logger that it will be formatted and that is
incorrect. In the 1.4-1.7 logging, message construction cost is delayed
until formatting which is as lazy as it gets in the logging world. Take
the workaround you suggest bellow. If you apply Object.toString() to
any of the arguments, then that cripples what you can do with custom
Filter or custom Formatter because you want to be able to access the
arguments in their original form and not the string representation.
Also, you always want to use the ResouceBundle assigned to the LogRecord
from the logger to do the localization. The msg supplier won't know
what that is at the time the lambda is created or I would have to
recreate code that the logger already does for me every time I want to
log something. It would!
be easie
r to do what we've done since 1.4 which is use a guard statement to
avoid evaluation of the expensive method call. Against this patch if I
use a lambda or a guard they will both evaluate the expensive call under
the same scenarios. Take the 'DiagnosisMessages::systemHealthStatus'
example from the API docs. Seems fine until you realize that someday
you might have to read the output of that statement in a log somewhere
or you want to create a filter that only shows when the system is
unhealthy. So you start to transform that example and realize that you
don't want to create a 'systemHealthStatusWithContextMsg' method because
it can't be localized during formatting. You don't want to simply
perform msg concatenation because that is bad practice and doesn't use
lambda. So skip using the lambda APIs because you can use the
parameterized logging with a guard statement and that allows you to
localize the message and or use the raw parameter data in a Filter to
determine which !
system va
lue has exceed some threshold without resorting to message parsing.
Parameters are always more useful than a preformatted string message.
Once you arrive here, there is no need for a message parameter to be
anything other than a message format pattern or a resource bundle key.
Both of those types of messages are string literals so I don't need a
Supplier. I think what would be more powerful and fitting patch would be
to overload all of the Logger.finest, Logger.finer, Logger.fine, etc.
like 'Logger.finer(String msg, Throwable thrown, Supplier... params)' or
use a sub-interface of Supplier. As long as the given
Supplier.toString() is implemented as: 'return String.valueof(get())'
then the existing logging API would format these lazy parameters the
same way and would properly delay the construction cost to only at the
time of formatting. Filters would be allowed access to the original
parameters through the supplier interface and the already established
localization in th!
e logging
API would still work. The established best practice of not creating on
the fly messages would still remain an enduring goal of the logging API.
Respectfully, Jason
For messages not just only for developer, they should be localized as
you suggested and in such cases, it is possible to achieve the laziness
via Object.toString(), less straightforward than using a lambda, but is
possible.
Cheers,
Henry
--
- DML
