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

[Ceki Gülcü]

The idea is to have technical discussions so that they are public. Thus, 
having it in Jira or on this mailing list is more or less equivalent. In 
particular because Jira comments are echoed on this mailing list as well.

As such, please see my comment in https://jira.qos.ch/browse/SLF4J-468


On 13/08/2019 23:11, Wessel van Norel wrote:
> 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)
> _______________________________________________
> slf4j-dev mailing list
> slf4j-dev at qos.ch
> http://mailman.qos.ch/mailman/listinfo/slf4j-dev
>