You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by rp...@apache.org on 2016/11/26 02:05:53 UTC
logging-log4j2 git commit: LOG4J2-1684 avoid creating temp objects
Repository: logging-log4j2
Updated Branches:
refs/heads/master 561f9a4f2 -> fe4abe31d
LOG4J2-1684 avoid creating temp objects
Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/fe4abe31
Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/fe4abe31
Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/fe4abe31
Branch: refs/heads/master
Commit: fe4abe31d410e49adbcdf5cda79f777bf3b19926
Parents: 561f9a4
Author: rpopma <rp...@apache.org>
Authored: Sat Nov 26 11:05:49 2016 +0900
Committer: rpopma <rp...@apache.org>
Committed: Sat Nov 26 11:05:49 2016 +0900
----------------------------------------------------------------------
.../apache/logging/log4j/message/StructuredDataMessage.java | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/fe4abe31/log4j-api/src/main/java/org/apache/logging/log4j/message/StructuredDataMessage.java
----------------------------------------------------------------------
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/message/StructuredDataMessage.java b/log4j-api/src/main/java/org/apache/logging/log4j/message/StructuredDataMessage.java
index 141098a..0de2920 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/message/StructuredDataMessage.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/StructuredDataMessage.java
@@ -225,8 +225,8 @@ public class StructuredDataMessage extends MapMessage implements StringBuilderFo
if (key.length() > MAX_LENGTH) {
throw new IllegalArgumentException("Structured data keys are limited to 32 characters. key: " + key);
}
- final char[] chars = key.toCharArray();
- for (final char c : chars) {
+ for (int i = 0; i < key.length(); i++) {
+ final char c = key.charAt(i);
if (c < '!' || c > '~' || c == '=' || c == ']' || c == '"') {
throw new IllegalArgumentException("Structured data keys must contain printable US ASCII characters" +
"and may not contain a space, =, ], or \"");
@@ -336,7 +336,8 @@ public class StructuredDataMessage extends MapMessage implements StringBuilderFo
@Override
public String getFormattedMessage(final String[] formats) {
if (formats != null && formats.length > 0) {
- for (final String format : formats) {
+ for (int i = 0; i < formats.length; i++) {
+ final String format = formats[i];
if (Format.XML.name().equalsIgnoreCase(format)) {
return asXml();
} else if (Format.FULL.name().equalsIgnoreCase(format)) {
Re: logging-log4j2 git commit: LOG4J2-1684 avoid creating temp objects
Posted by Remko Popma <re...@gmail.com>.
I guess it depends.
Obtaining the array is not free: it needs to be allocated, then initialized with zeros (Java lang spec), and finally the String contents are copied into it.
Once we have an array, accessing its elements directly will be faster than going through the String wrapper object since there is less pointer chasing.
Finally the cost of GC-ing the temp array is hard to measure.
Sent from my iPhone
> On 26 Nov 2016, at 12:11, Gary Gregory <ga...@gmail.com> wrote:
>
> But shouldn't the comparison really be using [] access as opposed to charAt()?
>
> Gary
>
>
>> On Nov 25, 2016 6:41 PM, "Remko Popma" <re...@gmail.com> wrote:
>> No this is actually faster.
>> I believe I created a benchmark for this that showed that for-each iteration is slower than indexed iteration.
>>
>> Sent from my iPhone
>>
>>> On 26 Nov 2016, at 11:29, Gary Gregory <ga...@gmail.com> wrote:
>>>
>>> Isn't this code slower? It feels like we are putting memory performance ahead of speed.
>>>
>>> Gary
>>>
>>> ---------- Forwarded message ----------
>>> From: <rp...@apache.org>
>>> Date: Nov 25, 2016 6:05 PM
>>> Subject: logging-log4j2 git commit: LOG4J2-1684 avoid creating temp objects
>>> To: <co...@logging.apache.org>
>>> Cc:
>>>
>>>> Repository: logging-log4j2
>>>> Updated Branches:
>>>> refs/heads/master 561f9a4f2 -> fe4abe31d
>>>>
>>>>
>>>> LOG4J2-1684 avoid creating temp objects
>>>>
>>>>
>>>> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
>>>> Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/fe4abe31
>>>> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/fe4abe31
>>>> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/fe4abe31
>>>>
>>>> Branch: refs/heads/master
>>>> Commit: fe4abe31d410e49adbcdf5cda79f777bf3b19926
>>>> Parents: 561f9a4
>>>> Author: rpopma <rp...@apache.org>
>>>> Authored: Sat Nov 26 11:05:49 2016 +0900
>>>> Committer: rpopma <rp...@apache.org>
>>>> Committed: Sat Nov 26 11:05:49 2016 +0900
>>>>
>>>> ----------------------------------------------------------------------
>>>> .../apache/logging/log4j/message/StructuredDataMessage.java | 7 ++++---
>>>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>>> ----------------------------------------------------------------------
>>>>
>>>>
>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/fe4abe31/log4j-api/src/main/java/org/apache/logging/log4j/message/StructuredDataMessage.java
>>>> ----------------------------------------------------------------------
>>>> diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/message/StructuredDataMessage.java b/log4j-api/src/main/java/org/apache/logging/log4j/message/StructuredDataMessage.java
>>>> index 141098a..0de2920 100644
>>>> --- a/log4j-api/src/main/java/org/apache/logging/log4j/message/StructuredDataMessage.java
>>>> +++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/StructuredDataMessage.java
>>>> @@ -225,8 +225,8 @@ public class StructuredDataMessage extends MapMessage implements StringBuilderFo
>>>> if (key.length() > MAX_LENGTH) {
>>>> throw new IllegalArgumentException("Structured data keys are limited to 32 characters. key: " + key);
>>>> }
>>>> - final char[] chars = key.toCharArray();
>>>> - for (final char c : chars) {
>>>> + for (int i = 0; i < key.length(); i++) {
>>>> + final char c = key.charAt(i);
>>>> if (c < '!' || c > '~' || c == '=' || c == ']' || c == '"') {
>>>> throw new IllegalArgumentException("Structured data keys must contain printable US ASCII characters" +
>>>> "and may not contain a space, =, ], or \"");
>>>> @@ -336,7 +336,8 @@ public class StructuredDataMessage extends MapMessage implements StringBuilderFo
>>>> @Override
>>>> public String getFormattedMessage(final String[] formats) {
>>>> if (formats != null && formats.length > 0) {
>>>> - for (final String format : formats) {
>>>> + for (int i = 0; i < formats.length; i++) {
>>>> + final String format = formats[i];
>>>> if (Format.XML.name().equalsIgnoreCase(format)) {
>>>> return asXml();
>>>> } else if (Format.FULL.name().equalsIgnoreCase(format)) {
>>>>
Re: logging-log4j2 git commit: LOG4J2-1684 avoid creating temp objects
Posted by Gary Gregory <ga...@gmail.com>.
But shouldn't the comparison really be using [] access as opposed to
charAt()?
Gary
On Nov 25, 2016 6:41 PM, "Remko Popma" <re...@gmail.com> wrote:
> No this is actually faster.
> I believe I created a benchmark for this that showed that for-each
> iteration is slower than indexed iteration.
>
> Sent from my iPhone
>
> On 26 Nov 2016, at 11:29, Gary Gregory <ga...@gmail.com> wrote:
>
> Isn't this code slower? It feels like we are putting memory performance
> ahead of speed.
>
> Gary
> ---------- Forwarded message ----------
> From: <rp...@apache.org>
> Date: Nov 25, 2016 6:05 PM
> Subject: logging-log4j2 git commit: LOG4J2-1684 avoid creating temp objects
> To: <co...@logging.apache.org>
> Cc:
>
> Repository: logging-log4j2
>> Updated Branches:
>> refs/heads/master 561f9a4f2 -> fe4abe31d
>>
>>
>> LOG4J2-1684 avoid creating temp objects
>>
>>
>> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
>> Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit
>> /fe4abe31
>> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/fe4abe31
>> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/fe4abe31
>>
>> Branch: refs/heads/master
>> Commit: fe4abe31d410e49adbcdf5cda79f777bf3b19926
>> Parents: 561f9a4
>> Author: rpopma <rp...@apache.org>
>> Authored: Sat Nov 26 11:05:49 2016 +0900
>> Committer: rpopma <rp...@apache.org>
>> Committed: Sat Nov 26 11:05:49 2016 +0900
>>
>> ----------------------------------------------------------------------
>> .../apache/logging/log4j/message/StructuredDataMessage.java | 7
>> ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>> ----------------------------------------------------------------------
>>
>>
>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/f
>> e4abe31/log4j-api/src/main/java/org/apache/logging/log4j/mes
>> sage/StructuredDataMessage.java
>> ----------------------------------------------------------------------
>> diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/message/StructuredDataMessage.java
>> b/log4j-api/src/main/java/org/apache/logging/log4j/message/S
>> tructuredDataMessage.java
>> index 141098a..0de2920 100644
>> --- a/log4j-api/src/main/java/org/apache/logging/log4j/message/S
>> tructuredDataMessage.java
>> +++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/S
>> tructuredDataMessage.java
>> @@ -225,8 +225,8 @@ public class StructuredDataMessage extends MapMessage
>> implements StringBuilderFo
>> if (key.length() > MAX_LENGTH) {
>> throw new IllegalArgumentException("Structured data keys
>> are limited to 32 characters. key: " + key);
>> }
>> - final char[] chars = key.toCharArray();
>> - for (final char c : chars) {
>> + for (int i = 0; i < key.length(); i++) {
>> + final char c = key.charAt(i);
>> if (c < '!' || c > '~' || c == '=' || c == ']' || c == '"') {
>> throw new IllegalArgumentException("Structured data
>> keys must contain printable US ASCII characters" +
>> "and may not contain a space, =, ], or \"");
>> @@ -336,7 +336,8 @@ public class StructuredDataMessage extends MapMessage
>> implements StringBuilderFo
>> @Override
>> public String getFormattedMessage(final String[] formats) {
>> if (formats != null && formats.length > 0) {
>> - for (final String format : formats) {
>> + for (int i = 0; i < formats.length; i++) {
>> + final String format = formats[i];
>> if (Format.XML.name().equalsIgnoreCase(format)) {
>> return asXml();
>> } else if (Format.FULL.name().equalsIgnoreCase(format))
>> {
>>
>>
Re: logging-log4j2 git commit: LOG4J2-1684 avoid creating temp objects
Posted by Remko Popma <re...@gmail.com>.
No this is actually faster.
I believe I created a benchmark for this that showed that for-each iteration is slower than indexed iteration.
Sent from my iPhone
> On 26 Nov 2016, at 11:29, Gary Gregory <ga...@gmail.com> wrote:
>
> Isn't this code slower? It feels like we are putting memory performance ahead of speed.
>
> Gary
>
> ---------- Forwarded message ----------
> From: <rp...@apache.org>
> Date: Nov 25, 2016 6:05 PM
> Subject: logging-log4j2 git commit: LOG4J2-1684 avoid creating temp objects
> To: <co...@logging.apache.org>
> Cc:
>
>> Repository: logging-log4j2
>> Updated Branches:
>> refs/heads/master 561f9a4f2 -> fe4abe31d
>>
>>
>> LOG4J2-1684 avoid creating temp objects
>>
>>
>> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
>> Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/fe4abe31
>> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/fe4abe31
>> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/fe4abe31
>>
>> Branch: refs/heads/master
>> Commit: fe4abe31d410e49adbcdf5cda79f777bf3b19926
>> Parents: 561f9a4
>> Author: rpopma <rp...@apache.org>
>> Authored: Sat Nov 26 11:05:49 2016 +0900
>> Committer: rpopma <rp...@apache.org>
>> Committed: Sat Nov 26 11:05:49 2016 +0900
>>
>> ----------------------------------------------------------------------
>> .../apache/logging/log4j/message/StructuredDataMessage.java | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>> ----------------------------------------------------------------------
>>
>>
>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/fe4abe31/log4j-api/src/main/java/org/apache/logging/log4j/message/StructuredDataMessage.java
>> ----------------------------------------------------------------------
>> diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/message/StructuredDataMessage.java b/log4j-api/src/main/java/org/apache/logging/log4j/message/StructuredDataMessage.java
>> index 141098a..0de2920 100644
>> --- a/log4j-api/src/main/java/org/apache/logging/log4j/message/StructuredDataMessage.java
>> +++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/StructuredDataMessage.java
>> @@ -225,8 +225,8 @@ public class StructuredDataMessage extends MapMessage implements StringBuilderFo
>> if (key.length() > MAX_LENGTH) {
>> throw new IllegalArgumentException("Structured data keys are limited to 32 characters. key: " + key);
>> }
>> - final char[] chars = key.toCharArray();
>> - for (final char c : chars) {
>> + for (int i = 0; i < key.length(); i++) {
>> + final char c = key.charAt(i);
>> if (c < '!' || c > '~' || c == '=' || c == ']' || c == '"') {
>> throw new IllegalArgumentException("Structured data keys must contain printable US ASCII characters" +
>> "and may not contain a space, =, ], or \"");
>> @@ -336,7 +336,8 @@ public class StructuredDataMessage extends MapMessage implements StringBuilderFo
>> @Override
>> public String getFormattedMessage(final String[] formats) {
>> if (formats != null && formats.length > 0) {
>> - for (final String format : formats) {
>> + for (int i = 0; i < formats.length; i++) {
>> + final String format = formats[i];
>> if (Format.XML.name().equalsIgnoreCase(format)) {
>> return asXml();
>> } else if (Format.FULL.name().equalsIgnoreCase(format)) {
>>
Fwd: logging-log4j2 git commit: LOG4J2-1684 avoid creating temp objects
Posted by Gary Gregory <ga...@gmail.com>.
Isn't this code slower? It feels like we are putting memory performance
ahead of speed.
Gary
---------- Forwarded message ----------
From: <rp...@apache.org>
Date: Nov 25, 2016 6:05 PM
Subject: logging-log4j2 git commit: LOG4J2-1684 avoid creating temp objects
To: <co...@logging.apache.org>
Cc:
Repository: logging-log4j2
> Updated Branches:
> refs/heads/master 561f9a4f2 -> fe4abe31d
>
>
> LOG4J2-1684 avoid creating temp objects
>
>
> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
> Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/
> commit/fe4abe31
> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/fe4abe31
> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/fe4abe31
>
> Branch: refs/heads/master
> Commit: fe4abe31d410e49adbcdf5cda79f777bf3b19926
> Parents: 561f9a4
> Author: rpopma <rp...@apache.org>
> Authored: Sat Nov 26 11:05:49 2016 +0900
> Committer: rpopma <rp...@apache.org>
> Committed: Sat Nov 26 11:05:49 2016 +0900
>
> ----------------------------------------------------------------------
> .../apache/logging/log4j/message/StructuredDataMessage.java | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/
> fe4abe31/log4j-api/src/main/java/org/apache/logging/log4j/
> message/StructuredDataMessage.java
> ----------------------------------------------------------------------
> diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/message/StructuredDataMessage.java
> b/log4j-api/src/main/java/org/apache/logging/log4j/message/
> StructuredDataMessage.java
> index 141098a..0de2920 100644
> --- a/log4j-api/src/main/java/org/apache/logging/log4j/message/
> StructuredDataMessage.java
> +++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/
> StructuredDataMessage.java
> @@ -225,8 +225,8 @@ public class StructuredDataMessage extends MapMessage
> implements StringBuilderFo
> if (key.length() > MAX_LENGTH) {
> throw new IllegalArgumentException("Structured data keys are
> limited to 32 characters. key: " + key);
> }
> - final char[] chars = key.toCharArray();
> - for (final char c : chars) {
> + for (int i = 0; i < key.length(); i++) {
> + final char c = key.charAt(i);
> if (c < '!' || c > '~' || c == '=' || c == ']' || c == '"') {
> throw new IllegalArgumentException("Structured data keys
> must contain printable US ASCII characters" +
> "and may not contain a space, =, ], or \"");
> @@ -336,7 +336,8 @@ public class StructuredDataMessage extends MapMessage
> implements StringBuilderFo
> @Override
> public String getFormattedMessage(final String[] formats) {
> if (formats != null && formats.length > 0) {
> - for (final String format : formats) {
> + for (int i = 0; i < formats.length; i++) {
> + final String format = formats[i];
> if (Format.XML.name().equalsIgnoreCase(format)) {
> return asXml();
> } else if (Format.FULL.name().equalsIgnoreCase(format)) {
>
>