[ 
https://issues.apache.org/jira/browse/LOG4J2-1874?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16049773#comment-16049773
 ] 

Roman Leventov commented on LOG4J2-1874:
----------------------------------------

Thanks for review!

1) Yes, it's a bug
2) OK, and I would also add a comment on that synchronized block, that it is 
unnecessary at the moment, but added for safety.

> If you agree with the above changes just comment, no need to update the pull 
> request.
You mean that you could fix it yourself when merging?

> Add ByteBufferDestionation.write(ByteBuffer) and write(byte[], int, int) 
> methods and call them from TextEncoderHelper whenever possible
> ---------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: LOG4J2-1874
>                 URL: https://issues.apache.org/jira/browse/LOG4J2-1874
>             Project: Log4j 2
>          Issue Type: Improvement
>            Reporter: Roman Leventov
>            Assignee: Remko Popma
>
> Existing ByteBufferDestination API: getByteBuffer() and drain() is designed 
> so that synchronization couldn't be avoided. This doesn't allow to implement 
> LOG4J2-928.
> Github PR: https://github.com/apache/logging-log4j2/pull/71
> Added methods: write(ByteBuffer data) and write(byte[] data, int offset, int 
> length) are designed so that they should care about synchronization 
> themselves, internally, if needed. They should also synchronize with possible 
> concurrent users of the synchronized getByteBuffer() + drain() API. 
> Nevertheless, it allows for ByteBufferDestination implementations to 
> implement write() methods without lock-free.
> TextEncoderHelper (hence StringBuilderEncoder, which delegates it's logic to 
> TextEncoderHelper) is changed so that it calls ByteBufferDestination.write() 
> whenever possible.  There is an expectation that most of encoded events fit 
> the thread-local buffers, and write() could be called instead of writing to 
> destination.getByteBuffer() with synchronization.
> The PR also includes a sanity improvement: uses ByteBuffer.arrayOffset() at 
> some places.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to