[GitHub] logging-log4j2 pull request #205: [LOG4J2-2399] support for async formatting...

2018-08-09 Thread bjlaub
Github user bjlaub commented on a diff in the pull request:

https://github.com/apache/logging-log4j2/pull/205#discussion_r208965737
  
--- Diff: 
log4j-api/src/main/java/org/apache/logging/log4j/message/ReusableParameterizedMessage.java
 ---
@@ -355,4 +352,35 @@ public void clear() { // LOG4J2-1583
 messagePattern = null;
 throwable = null;
 }
+
+@Override
+public MessageContentFormatter getMessageContentFormatter() {
+return formatter;
+}
+
+private static final MessageContentFormatter formatter = new 
MessageContentFormatter() {
+private final ThreadLocal indices = new ThreadLocal<>();
--- End diff --

see 
https://github.com/apache/logging-log4j2/pull/208/commits/fc8ed21311d3a428b5a673a1b289ea2624e60cdb
 in #208 


---


[GitHub] logging-log4j2 pull request #205: [LOG4J2-2399] support for async formatting...

2018-08-09 Thread bjlaub
Github user bjlaub commented on a diff in the pull request:

https://github.com/apache/logging-log4j2/pull/205#discussion_r208965807
  
--- Diff: 
log4j-core/src/main/java/org/apache/logging/log4j/core/impl/MutableLogEvent.java
 ---
@@ -204,15 +206,19 @@ public void setLoggerName(final String loggerName) {
 @Override
 public Message getMessage() {
 if (message == null) {
-return messageText == null ? EMPTY : this;
+return (messageText == null && messageContentFormatter == 
null) ? EMPTY : this;
 }
 return message;
 }
 
 public void setMessage(final Message msg) {
 if (msg instanceof ReusableMessage) {
 final ReusableMessage reusable = (ReusableMessage) msg;
-reusable.formatTo(getMessageTextForWriting());
+if (Constants.FORMAT_MESSAGES_IN_BACKGROUND && msg instanceof 
MessageContentFormatterProvider) {
+this.messageContentFormatter = 
((MessageContentFormatterProvider) msg).getMessageContentFormatter();
--- End diff --

see 
https://github.com/apache/logging-log4j2/pull/208/commits/fc8ed21311d3a428b5a673a1b289ea2624e60cdb
 in #208 


---


[GitHub] logging-log4j2 pull request #205: [LOG4J2-2399] support for async formatting...

2018-08-09 Thread bjlaub
Github user bjlaub commented on a diff in the pull request:

https://github.com/apache/logging-log4j2/pull/205#discussion_r208965448
  
--- Diff: 
log4j-api/src/main/java/org/apache/logging/log4j/message/ReusableParameterizedMessage.java
 ---
@@ -355,4 +352,35 @@ public void clear() { // LOG4J2-1583
 messagePattern = null;
 throwable = null;
 }
+
+@Override
+public MessageContentFormatter getMessageContentFormatter() {
+return formatter;
+}
+
+private static final MessageContentFormatter formatter = new 
MessageContentFormatter() {
+private final ThreadLocal indices = new ThreadLocal<>();
+
+private int computeIndices(String messagePattern) {
+int[] result = indices.get();
+if (result == null) {
+result = new int[256];
+indices.set(result);
+}
+return ReusableParameterizedMessage.count(messagePattern, 
result);
+}
+
+@Override
+public void formatTo(String formatString, Object[] parameters, int 
parameterCount, StringBuilder buffer) {
+int placeholderCount = computeIndices(formatString);
+int usedCount = Math.min(placeholderCount, parameterCount);
+int[] computedIndices = indices.get();
--- End diff --

see 
https://github.com/apache/logging-log4j2/pull/208/commits/e015b4aab968a2aa20dc9a40082d98cfe63528cb
 in #208 


---


[GitHub] logging-log4j2 pull request #205: [LOG4J2-2399] support for async formatting...

2018-08-08 Thread cakofony
Github user cakofony commented on a diff in the pull request:

https://github.com/apache/logging-log4j2/pull/205#discussion_r208702264
  
--- Diff: 
log4j-api/src/main/java/org/apache/logging/log4j/message/ReusableParameterizedMessage.java
 ---
@@ -355,4 +352,35 @@ public void clear() { // LOG4J2-1583
 messagePattern = null;
 throwable = null;
 }
+
+@Override
+public MessageContentFormatter getMessageContentFormatter() {
+return formatter;
+}
+
+private static final MessageContentFormatter formatter = new 
MessageContentFormatter() {
+private final ThreadLocal indices = new ThreadLocal<>();
--- End diff --

What happens to these values if nested logging occurs in 
ParameterFormatter.formatMessage2? See 
[LOG4J2-1583](https://issues.apache.org/jira/browse/LOG4J2-1583) referenced on 
`ReusableParameterizedMessage.reserved`. We should write a test for this case.


---


[GitHub] logging-log4j2 pull request #205: [LOG4J2-2399] support for async formatting...

2018-08-08 Thread cakofony
Github user cakofony commented on a diff in the pull request:

https://github.com/apache/logging-log4j2/pull/205#discussion_r208699061
  
--- Diff: 
log4j-api/src/main/java/org/apache/logging/log4j/message/ReusableParameterizedMessage.java
 ---
@@ -125,12 +123,23 @@ private void init(final String messagePattern, final 
int argCount, final Object[
 this.varargs = null;
 this.messagePattern = messagePattern;
 this.argCount = argCount;
-final int placeholderCount = count(messagePattern, indices);
-initThrowable(paramArray, argCount, placeholderCount);
-this.usedCount = Math.min(placeholderCount, argCount);
+
+int usedParams = count(messagePattern, null);
+if (usedParams < argCount && paramArray[argCount - 1] instanceof 
Throwable) {
+this.throwable = (Throwable) paramArray[argCount - 1];
+} else {
+this.throwable = null;
+}
--- End diff --

I wonder if we should check `argCount > 0 && paramArray[argCount - 1] 
instanceof Throwable` prior to invoking count? Could be worth a benchmark, 
either way one iteration over a format string isn't very expensive.


---


[GitHub] logging-log4j2 pull request #205: [LOG4J2-2399] support for async formatting...

2018-08-08 Thread cakofony
Github user cakofony commented on a diff in the pull request:

https://github.com/apache/logging-log4j2/pull/205#discussion_r208700750
  
--- Diff: 
log4j-api/src/main/java/org/apache/logging/log4j/message/ReusableParameterizedMessage.java
 ---
@@ -355,4 +352,35 @@ public void clear() { // LOG4J2-1583
 messagePattern = null;
 throwable = null;
 }
+
+@Override
+public MessageContentFormatter getMessageContentFormatter() {
+return formatter;
+}
+
+private static final MessageContentFormatter formatter = new 
MessageContentFormatter() {
+private final ThreadLocal indices = new ThreadLocal<>();
+
+private int computeIndices(String messagePattern) {
+int[] result = indices.get();
+if (result == null) {
+result = new int[256];
+indices.set(result);
+}
+return ReusableParameterizedMessage.count(messagePattern, 
result);
+}
+
+@Override
+public void formatTo(String formatString, Object[] parameters, int 
parameterCount, StringBuilder buffer) {
+int placeholderCount = computeIndices(formatString);
+int usedCount = Math.min(placeholderCount, parameterCount);
+int[] computedIndices = indices.get();
--- End diff --

Can we avoid the second `ThreadLocal.get` here? Might be easiest if we do 
the logic from `computeIndices` in this method 


---


[GitHub] logging-log4j2 pull request #205: [LOG4J2-2399] support for async formatting...

2018-08-08 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/logging-log4j2/pull/205


---


[GitHub] logging-log4j2 pull request #205: [LOG4J2-2399] support for async formatting...

2018-08-08 Thread bjlaub
GitHub user bjlaub opened a pull request:

https://github.com/apache/logging-log4j2/pull/205

[LOG4J2-2399] support for async formatting on reusable messages

Adds support for deferring message content formatting on certain
ReusableMessage implementations to a background thread when async
logging is enabled. This change adds the ability for log events to
retain an instance of a formatter for a message so that the message
content can be formatted later, after the message has already been
recycled.

This is an effort to optimize performance for use cases where formatting
on the logging thread is unnecessary or undesired.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/bjlaub/logging-log4j2 
async-background-formatting

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/logging-log4j2/pull/205.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #205


commit ddae40953cfe24232f58f23e90a417623a260853
Author: Brian Laub 
Date:   2018-08-08T16:13:35Z

[LOG4J2-2399] support for async formatting on reusable messages

Adds support for deferring message content formatting on certain
ReusableMessage implementations to a background thread when async
logging is enabled. This change adds the ability for log events to
retain an instance of a formatter for a message so that the message
content can be formatted later, after the message has already been
recycled.

This is an effort to optimize performance for use cases where formatting
on the logging thread is unnecessary or undesired.




---