[slf4j-dev] MessageFormatter: small performance improvement for 1 and 2 object parameter cases?

[Wessel van Norel]

Hi all,

When I was looking at the issue with the Throwable parameter in the
EventRecordingLogger (SLF4J-466[1]) I also stumbled upon an issue
within the MessageFormatter. I opened SLF4J-468[2] for that.

Ceki asked me to explain this issue to the list, since a change in
this class is not something we should do lightly.

There are two cases.

Case 1:
In case you pass one object to the format method[3] it will put that
object in an Object[] and then the next method will call yet another
method to see if that Object[]'s last element is a Throwable. And if
so it will create a new Object[] containing 0 elements and it will try
to do the formatting logic, even though there is nothing to format.

My proposed changes:

2.0.0:
  - Directly call the actual message formatting function with an
Object[1] argArray and null throwable.[4]

The downside of this change is that if the one object format message
is called with a Throwable you no longer get 0 arguments and a
Stacktrace, you get 1 argument and no Stacktrace. But why would you
call the format method if you already know there is nothing to format?

1.7.x:
 - Add an instanceof check to the object argument
 - Call the actual formatting function with either null as argArray or
null as throwable and the actual argument on the other.
 - Extend the argArray == null check with || argArray.length == 0 and
change the return to return new FormattingTuple(messagePattern, null,
throwable);

I think this last change should also be in the 2.0.0 branch, but it's
not strictly required for this issue. But now the Throwable might get
lost in case someone calls the actual formatting function with an
empty argument array.


Case 2:
In case you pass two objects to the format method[5] it will put both
objects in an Object[] and then the next method will call yet another
method to see if the last Object in this Object[] is a Throwable. And
if so it will create a new Object[] containing only the first element,
which it copies from the original Object[]. And then it will do the
normal formatting logic.

My proposed changes, for both branches:
- Add an instanceof check to the second object argument
- Call the actual formatting function with either an Object[2]
argArray and null throwable, or an Object[1] argArray and the
throwable.


Of course, it would even be nicer to not have to do the get last
parameter and check if it's a Throwable and if so do some other magic
then in the other case. But as a user of the logging framework the
current way of working is very convenient and logical. So I guess we
cannot change that in the current API but only in the new fluent API?

I hope I explained the issue clearly.

Kind regards,
Wessel

[1] https://jira.qos.ch/browse/SLF4J-466
[2] https://jira.qos.ch/browse/SLF4J-468
[3] https://www.slf4j.org/apidocs/org/slf4j/helpers/MessageFormatter.html#format(java.lang.String,java.lang.Object)
[4] https://www.slf4j.org/apidocs/org/slf4j/helpers/MessageFormatter.html#arrayFormat(java.lang.String,java.lang.Object%5B%5D,java.lang.Throwable)
[5] https://www.slf4j.org/apidocs/org/slf4j/helpers/MessageFormatter.html#format(java.lang.String,java.lang.Object,java.lang.Object)