You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4j-dev@logging.apache.org by Gary Gregory <ga...@gmail.com> on 2017/01/26 18:32:55 UTC

Making copies of LogEvent to avoid no-GC problems

Hi All:

Just like in our SMTP appender, I have a custom Appender which needs to
track LogEvents. To do this without falling prey to all of our no-GC epic
cleverness, the SMTP appender does this:

    public void add(LogEvent event) {
        if (event instanceof Log4jLogEvent && event.getMessage() instanceof
ReusableMessage) {
            ((Log4jLogEvent) event).makeMessageImmutable();
        } else if (event instanceof MutableLogEvent) {
            event = ((MutableLogEvent) event).createMemento();
        }
        buffer.add(event);
    }

The code in my Appender is functionally identical, I just use a different
buffer type.

This kind of Appender code is impossible to write correctly first IMO.

My first implementation was just:

    public void add(LogEvent event) {
        buffer.add(event);
    }

After a bug hunt by a colleague of mine, I thought the fix for our use case
was 'simple':

    public void add(LogEvent event) {
        buffer.add(event instanceof MutableLogEvent ? ((MutableLogEvent)
event).createMemento() : event);
    }

But that was not complete!

Our Appender now does the same thing the SMTP Appender does.

Should we allow LogEvent to express this more cleanly? Maybe:

    public void add(LogEvent event) {
        buffer.add(event.asImmutableLogEvent());
    }

Where MutableLogEvent.asImmutableLogEvent() just calls createMemento() and
LogEvent changes its internal state by calling makeMessageImmutable() and
returns 'this' since creating a new LogEvent is not needed.

Thoughts?

Gary

-- 
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition
<https://www.amazon.com/gp/product/1617290459/ref=as_li_tl?ie=UTF8&camp=1789&creative=9325&creativeASIN=1617290459&linkCode=as2&tag=garygregory-20&linkId=cadb800f39946ec62ea2b1af9fe6a2b8>

<http:////ir-na.amazon-adsystem.com/e/ir?t=garygregory-20&l=am2&o=1&a=1617290459>
JUnit in Action, Second Edition
<https://www.amazon.com/gp/product/1935182021/ref=as_li_tl?ie=UTF8&camp=1789&creative=9325&creativeASIN=1935182021&linkCode=as2&tag=garygregory-20&linkId=31ecd1f6b6d1eaf8886ac902a24de418%22>

<http:////ir-na.amazon-adsystem.com/e/ir?t=garygregory-20&l=am2&o=1&a=1935182021>
Spring Batch in Action
<https://www.amazon.com/gp/product/1935182951/ref=as_li_tl?ie=UTF8&camp=1789&creative=9325&creativeASIN=1935182951&linkCode=%7B%7BlinkCode%7D%7D&tag=garygregory-20&linkId=%7B%7Blink_id%7D%7D%22%3ESpring+Batch+in+Action>
<http:////ir-na.amazon-adsystem.com/e/ir?t=garygregory-20&l=am2&o=1&a=1935182951>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory

Re: Making copies of LogEvent to avoid no-GC problems

Posted by Gary Gregory <ga...@gmail.com>.
On Thu, Jan 26, 2017 at 2:45 PM, Remko Popma <re...@gmail.com> wrote:

> Yes, makes sense to me.
> Remko
>

I have a patch attached here:
https://issues.apache.org/jira/browse/LOG4J2-1807

But I'd like to be able to build on Windows before committing it...

Gary


>
> Sent from my iPhone
>
> On Jan 27, 2017, at 3:32, Gary Gregory <ga...@gmail.com> wrote:
>
> Hi All:
>
> Just like in our SMTP appender, I have a custom Appender which needs to
> track LogEvents. To do this without falling prey to all of our no-GC epic
> cleverness, the SMTP appender does this:
>
>     public void add(LogEvent event) {
>         if (event instanceof Log4jLogEvent && event.getMessage()
> instanceof ReusableMessage) {
>             ((Log4jLogEvent) event).makeMessageImmutable();
>         } else if (event instanceof MutableLogEvent) {
>             event = ((MutableLogEvent) event).createMemento();
>         }
>         buffer.add(event);
>     }
>
> The code in my Appender is functionally identical, I just use a different
> buffer type.
>
> This kind of Appender code is impossible to write correctly first IMO.
>
> My first implementation was just:
>
>     public void add(LogEvent event) {
>         buffer.add(event);
>     }
>
> After a bug hunt by a colleague of mine, I thought the fix for our use
> case was 'simple':
>
>     public void add(LogEvent event) {
>         buffer.add(event instanceof MutableLogEvent ? ((MutableLogEvent)
> event).createMemento() : event);
>     }
>
> But that was not complete!
>
> Our Appender now does the same thing the SMTP Appender does.
>
> Should we allow LogEvent to express this more cleanly? Maybe:
>
>     public void add(LogEvent event) {
>         buffer.add(event.asImmutableLogEvent());
>     }
>
> Where MutableLogEvent.asImmutableLogEvent() just calls createMemento()
> and LogEvent changes its internal state by calling makeMessageImmutable()
> and returns 'this' since creating a new LogEvent is not needed.
>
> Thoughts?
>
> Gary
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@apache.org
> Java Persistence with Hibernate, Second Edition
> <https://www.amazon.com/gp/product/1617290459/ref=as_li_tl?ie=UTF8&camp=1789&creative=9325&creativeASIN=1617290459&linkCode=as2&tag=garygregory-20&linkId=cadb800f39946ec62ea2b1af9fe6a2b8>
>
> <http:////ir-na.amazon-adsystem.com/e/ir?t=garygregory-20&l=am2&o=1&a=1617290459>
> JUnit in Action, Second Edition
> <https://www.amazon.com/gp/product/1935182021/ref=as_li_tl?ie=UTF8&camp=1789&creative=9325&creativeASIN=1935182021&linkCode=as2&tag=garygregory-20&linkId=31ecd1f6b6d1eaf8886ac902a24de418%22>
>
> <http:////ir-na.amazon-adsystem.com/e/ir?t=garygregory-20&l=am2&o=1&a=1935182021>
> Spring Batch in Action
> <https://www.amazon.com/gp/product/1935182951/ref=as_li_tl?ie=UTF8&camp=1789&creative=9325&creativeASIN=1935182951&linkCode=%7B%7BlinkCode%7D%7D&tag=garygregory-20&linkId=%7B%7Blink_id%7D%7D%22%3ESpring+Batch+in+Action>
> <http:////ir-na.amazon-adsystem.com/e/ir?t=garygregory-20&l=am2&o=1&a=1935182951>
> Blog: http://garygregory.wordpress.com
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory
>
>


-- 
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition
<https://www.amazon.com/gp/product/1617290459/ref=as_li_tl?ie=UTF8&camp=1789&creative=9325&creativeASIN=1617290459&linkCode=as2&tag=garygregory-20&linkId=cadb800f39946ec62ea2b1af9fe6a2b8>

<http:////ir-na.amazon-adsystem.com/e/ir?t=garygregory-20&l=am2&o=1&a=1617290459>
JUnit in Action, Second Edition
<https://www.amazon.com/gp/product/1935182021/ref=as_li_tl?ie=UTF8&camp=1789&creative=9325&creativeASIN=1935182021&linkCode=as2&tag=garygregory-20&linkId=31ecd1f6b6d1eaf8886ac902a24de418%22>

<http:////ir-na.amazon-adsystem.com/e/ir?t=garygregory-20&l=am2&o=1&a=1935182021>
Spring Batch in Action
<https://www.amazon.com/gp/product/1935182951/ref=as_li_tl?ie=UTF8&camp=1789&creative=9325&creativeASIN=1935182951&linkCode=%7B%7BlinkCode%7D%7D&tag=garygregory-20&linkId=%7B%7Blink_id%7D%7D%22%3ESpring+Batch+in+Action>
<http:////ir-na.amazon-adsystem.com/e/ir?t=garygregory-20&l=am2&o=1&a=1935182951>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory

Re: Making copies of LogEvent to avoid no-GC problems

Posted by Remko Popma <re...@gmail.com>.
Yes, makes sense to me. 
Remko 

Sent from my iPhone

> On Jan 27, 2017, at 3:32, Gary Gregory <ga...@gmail.com> wrote:
> 
> Hi All:
> 
> Just like in our SMTP appender, I have a custom Appender which needs to track LogEvents. To do this without falling prey to all of our no-GC epic cleverness, the SMTP appender does this:
> 
>     public void add(LogEvent event) {
>         if (event instanceof Log4jLogEvent && event.getMessage() instanceof ReusableMessage) {
>             ((Log4jLogEvent) event).makeMessageImmutable();
>         } else if (event instanceof MutableLogEvent) {
>             event = ((MutableLogEvent) event).createMemento();
>         }
>         buffer.add(event);
>     }
> 
> The code in my Appender is functionally identical, I just use a different buffer type.
> 
> This kind of Appender code is impossible to write correctly first IMO.
> 
> My first implementation was just:
> 
>     public void add(LogEvent event) {
>         buffer.add(event);
>     }
> 
> After a bug hunt by a colleague of mine, I thought the fix for our use case was 'simple':
> 
>     public void add(LogEvent event) {
>         buffer.add(event instanceof MutableLogEvent ? ((MutableLogEvent) event).createMemento() : event);
>     }
> 
> But that was not complete! 
> 
> Our Appender now does the same thing the SMTP Appender does.
> 
> Should we allow LogEvent to express this more cleanly? Maybe:
> 
>     public void add(LogEvent event) {
>         buffer.add(event.asImmutableLogEvent());
>     }
> 
> Where MutableLogEvent.asImmutableLogEvent() just calls createMemento() and LogEvent changes its internal state by calling makeMessageImmutable() and returns 'this' since creating a new LogEvent is not needed.
> 
> Thoughts?
> 
> Gary
> 
> -- 
> E-Mail: garydgregory@gmail.com | ggregory@apache.org 
> Java Persistence with Hibernate, Second Edition 
> JUnit in Action, Second Edition 
> Spring Batch in Action 
> Blog: http://garygregory.wordpress.com 
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory