What you have done is separate the format from the data, If you look at
AbstractLogger you will notice that the logging information (format string and
parameters) is all grouped together . The LogEvent only contains a Message, not
format strings and parameters.
To do what you are suggesting, logger.debug would have to retrieve the Message
and then do something with the params. It can't modify the Message object by
storing the params in it as that isn't thread-safe, and passing the parameters
to the LogEvent leads to other problems.
What might make more sense would to use a MessageFactory to bypass the cost of
reflection. But then the API would need to reference the factory somehow.
Ralph
On Oct 5, 2012, at 1:11 PM, Paul Benedict wrote:
> IMO, I thought all Message implements should be stateless. Maybe that should
> be part of the Message interface contract anyway, regardless of my idea.
>
> Paul
>
> On Fri, Oct 5, 2012 at 3:06 PM, Gary Gregory <garydgreg...@gmail.com> wrote:
> That's clever but... when happens when multiple threads call run(String)?
> We'd need to make sure all Message implementations are stateless.
>
> I'm not sure I'd want to keep both places in the code in sync too. Every time
> I want to fiddle with a message format, I have to make sure both spots match.
> Seems to painful to me. An interesting thought though, but it does not make
> my life easier as a v2 user...
>
> Gary
>
> On Fri, Oct 5, 2012 at 2:53 PM, Paul Benedict <pbened...@apache.org> wrote:
> After reading this email, it occurred to me that Messages/Formats are
> typically static. Once instantiated, there is really no need to instantiate
> it over and over.
>
> Rather than specifying the Class<?>, perhaps there should be a registration
> feature. Maybe like this:
>
> public class MyApp {
> public static Logger logger;
> static {
> logger.registerMessage("messagekey", new StringFormattedMessage("%s"));
> }
>
> public void run(String param) {
> logger.debug("messagekey", param);
> }
> }
>
> Paul
>
>
> On Fri, Oct 5, 2012 at 1:03 PM, Ralph Goers <ralph.go...@dslextreme.com>
> wrote:
> I updated ReflectionComparison to include comparing creating the object via
> "new" and via reflection.
>
> Timer NewObject stopped. Elapsed time: 0.019149000 seconds Average per
> iteration: 0.000000019 seconds
> Timer ReflectionObject stopped. Elapsed time: 0.171598000 seconds Average per
> iteration: 0.000000171 seconds
>
> So reflection definitely is noticeably slower, but this is only going to
> occur after filtering has passed and is probably a good tradeoff for avoiding
> calling "new" in the application until it is actually needed. For example,
> there would probably be hundreds of logging calls that wouldn't create an
> object because filtering didn't pass, thus compensating for the extra cost of
> allocating the objects and immediately discarding them. OTOH, doing
>
> if (logger.isDebugEnabled()) {
> logger.debug(new StringFormattedMessage(format, param));
> }
>
> would probably perform better than
>
> logger.debug(StringFormattedMessage.class, format, param);
>
> Ralph
>
>
> On Oct 5, 2012, at 8:35 AM, Gary Gregory wrote:
>
>> On Thu, Oct 4, 2012 at 10:54 PM, Ralph Goers <rgo...@apache.org> wrote:
>> Yup. That is what I would expect. However, it probably requires a Message
>> that has a constructor that accepts a Throwable and an object array, which
>> means we would probably want a new interface as a marker.
>>
>> So would reflection be used at runtime to instantiate the message? Wouldn't
>> that be a performance hit?
>>
>> We could just provide APIs in AbstractLogger for the built-in message
>> classes that do the instance creation w/o reflection.
>>
>> Thoughts?
>>
>> Gary
>>
>>
>> Ralph
>>
>> Sent from my iPad
>>
>> On Oct 4, 2012, at 7:18 PM, Paul Benedict <pbened...@apache.org> wrote:
>>
>>> Ooops. I meant this:
>>>
>>> logger.debug(Class<? extends Message> m, Throwable t, Object...
>>> messageParams);
>>>
>>> The point was to pass in the Class of the Message so it doesn't get
>>> instantiated unless logging is going to occur.
>>>
>>> Paul
>>>
>>> On Thu, Oct 4, 2012 at 9:12 PM, Gary Gregory <garydgreg...@gmail.com> wrote:
>>> On Thu, Oct 4, 2012 at 10:06 PM, Paul Benedict <pbened...@apache.org> wrote:
>>> On Thu, Oct 4, 2012 at 7:24 PM, Gary Gregory <garydgreg...@gmail.com> wrote:
>>> On Thu, Oct 4, 2012 at 5:55 PM, Paul Benedict <pbened...@apache.org> wrote:
>>> Ralph,
>>>
>>> This is actually a discussion you and I had a while back when I was trying
>>> to figure out how to use String.format(). I like the model now of
>>> specifying the message class... however...
>>>
>>> It does seem a bit unseemly to instantiate an xxxMessage object that may
>>> never get used. I'd rather just pass in the Class<?> and let the logger
>>> instantiate it only if it is going to log something. The only downside is
>>> then configuring the actual class.
>>>
>>> Thoughts?
>>>
>>> So instead of:
>>>
>>> this.logger.debug(new StringFormattedMessage(format, values), t);
>>>
>>> I would do:
>>>
>>> this.logger.debug(StringFormattedMessage.class, t, format, values);
>>>
>>>
>>> I was thinking of adding this signature:
>>> logger.debug(Message m, Throwable t, Object... messageParams);
>>>
>>> Thoughts?
>>>
>>> Pardon me for being dense, but how does that help in the case of my
>>> examples?
>>>
>>> Thank you in advance for clarifying,
>>> Gary
>>>
>>> Paul
>>>
>>>
>>>
>>> --
>>> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org
>>> JUnit in Action, 2nd Ed: http://bit.ly/ECvg0
>>> Spring Batch in Action: http://bit.ly/bqpbCK
>>> Blog: http://garygregory.wordpress.com
>>> Home: http://garygregory.com/
>>> Tweet! http://twitter.com/GaryGregory
>>>
>>
>>
>>
>> --
>> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org
>> JUnit in Action, 2nd Ed: http://bit.ly/ECvg0
>> Spring Batch in Action: http://bit.ly/bqpbCK
>> Blog: http://garygregory.wordpress.com
>> Home: http://garygregory.com/
>> Tweet! http://twitter.com/GaryGregory
>
>
>
>
>
> --
> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org
> JUnit in Action, 2nd Ed: http://bit.ly/ECvg0
> Spring Batch in Action: http://bit.ly/bqpbCK
> Blog: http://garygregory.wordpress.com
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory
>
