[GitHub] logging-log4j2 pull request #205: [LOG4J2-2399] support for async formatting...
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...
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...
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...
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...
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...
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...
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...
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. ---