You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2021/08/03 20:16:40 UTC
[GitHub] [logging-log4j2] carterkozak opened a new pull request #555: Improve worst-case NameAbbreviator performance
carterkozak opened a new pull request #555:
URL: https://github.com/apache/logging-log4j2/pull/555
Previously the entire input string was written to the output
buffer, then small sections were deleted or replaced within that
string. This meant that for each section that was abbreviated,
nearly all following characters from subsequent sections would
be migrated, resulting in O(n^2) performance.
Now we read from the existing input String, and write sections
to the output buffer piece-by-piece. For small strings this may
result in more pointer-hopping (may cost ~tens of nanoseconds) while
larger values with many abbreviated sections are substantially
less expensive. I would expect large "enterprise-grade" class
names to perform better than previously.
Note that the new approach may result in multiple StringBuilder
resize operations in the worst case, where previously writing
the original string to the output would have caused it to grow
all at once. In most cases we attempt to initialize builders
with a reasonable size, so I opted not to add an `ensureCapacity`
given the estimated size may be substantially larger than we need.
Benchmarks with this change:
```
NamePatternConverterBenchmark.benchNamePatternConverter org.bogus.hokus.pokus.org.bogus.hokus.pokus.org.bogus.hokus.pokus.RetroEncabulatorFactorySingleton avgt 3 202.992 ± 1.584 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter org.bogus.hokus.pokus.Clazz1 avgt 3 76.517 ± 2.979 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter com.bogus.hokus.pokus.Clazz2 avgt 3 75.369 ± 3.217 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter edu.bogus.hokus.pokus.a.Clazz3 avgt 3 97.546 ± 12.164 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter de.bogus.hokus.b.Clazz4 avgt 3 82.240 ± 14.489 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter jp.bogus.c.Clazz5 avgt 3 65.274 ± 3.938 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter cn.d.Clazz6 avgt 3 50.239 ± 0.456 ns/op
```
Previous results:
```
NamePatternConverterBenchmark.benchNamePatternConverter org.bogus.hokus.pokus.org.bogus.hokus.pokus.org.bogus.hokus.pokus.RetroEncabulatorFactorySingleton avgt 3 209.477 ± 62.262 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter org.bogus.hokus.pokus.Clazz1 avgt 3 76.083 ± 8.123 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter com.bogus.hokus.pokus.Clazz2 avgt 3 76.109 ± 19.293 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter edu.bogus.hokus.pokus.a.Clazz3 avgt 3 91.913 ± 46.029 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter de.bogus.hokus.b.Clazz4 avgt 3 74.703 ± 28.086 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter jp.bogus.c.Clazz5 avgt 3 59.788 ± 7.298 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter cn.d.Clazz6 avgt 3 41.092 ± 19.660 ns/op
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] carterkozak closed pull request #555: Improve worst-case NameAbbreviator performance
Posted by GitBox <gi...@apache.org>.
carterkozak closed pull request #555:
URL: https://github.com/apache/logging-log4j2/pull/555
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] carterkozak commented on pull request #555: Improve worst-case NameAbbreviator performance
Posted by GitBox <gi...@apache.org>.
carterkozak commented on pull request #555:
URL: https://github.com/apache/logging-log4j2/pull/555#issuecomment-969096581
@rgoers I'll merge this shortly, it doesn't make a tremendous difference either way, and I'm not worried if it doesn't make the 2.15 release.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] schlosna commented on a change in pull request #555: Improve worst-case NameAbbreviator performance
Posted by GitBox <gi...@apache.org>.
schlosna commented on a change in pull request #555:
URL: https://github.com/apache/logging-log4j2/pull/555#discussion_r683117411
##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/NameAbbreviator.java
##########
@@ -285,40 +285,40 @@ public PatternAbbreviatorFragment(
/**
* Abbreviate element of name.
*
- * @param buf buffer to receive element.
- * @param startPos starting index of name element.
- * @return starting index of next element.
+ * @param input input string which is being written to the output {@code buf}.
+ * @param inputIndex starting index of name element in the {@code input} string.
+ * @param buf buffer to receive element.
+ * @return starting index of next element.
*/
- public int abbreviate(final StringBuilder buf, final int startPos) {
- final int start = (startPos < 0) ? 0 : startPos;
- final int max = buf.length();
- int nextDot = -1;
- for (int i = start; i < max; i++) {
- if (buf.charAt(i) == '.') {
- nextDot = i;
- break;
- }
+ int abbreviate(final String input, final int inputIndex, final StringBuilder buf) {
+ // ckozak: indexOf with a string is intentional. indexOf(String, int) appears to
+ // have optimizations that don't apply to indexOf(char, int)
+ // which result in a >10% performance improvement in some
+ // NamePatternConverterBenchmark cases. This should be re-evaluated with
+ // future java releases.
+ int nextDot = input.indexOf(".", inputIndex);
Review comment:
Likely due to difference with intrinsic for `indexOf` with `String`, while the `char` version is not intrinsified until OpenJDK 16+, see https://bugs.openjdk.java.net/browse/JDK-8173585 , https://github.com/openjdk/jdk/commit/f71e8a61 and https://mail.openjdk.java.net/pipermail/jdk9-dev/2017-January/005539.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] carterkozak commented on pull request #555: Improve worst-case NameAbbreviator performance
Posted by GitBox <gi...@apache.org>.
carterkozak commented on pull request #555:
URL: https://github.com/apache/logging-log4j2/pull/555#issuecomment-969120027
Merged
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] carterkozak commented on a change in pull request #555: Improve worst-case NameAbbreviator performance
Posted by GitBox <gi...@apache.org>.
carterkozak commented on a change in pull request #555:
URL: https://github.com/apache/logging-log4j2/pull/555#discussion_r683429654
##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/NameAbbreviator.java
##########
@@ -285,40 +285,40 @@ public PatternAbbreviatorFragment(
/**
* Abbreviate element of name.
*
- * @param buf buffer to receive element.
- * @param startPos starting index of name element.
- * @return starting index of next element.
+ * @param input input string which is being written to the output {@code buf}.
+ * @param inputIndex starting index of name element in the {@code input} string.
+ * @param buf buffer to receive element.
+ * @return starting index of next element.
*/
- public int abbreviate(final StringBuilder buf, final int startPos) {
- final int start = (startPos < 0) ? 0 : startPos;
- final int max = buf.length();
- int nextDot = -1;
- for (int i = start; i < max; i++) {
- if (buf.charAt(i) == '.') {
- nextDot = i;
- break;
- }
+ int abbreviate(final String input, final int inputIndex, final StringBuilder buf) {
+ // ckozak: indexOf with a string is intentional. indexOf(String, int) appears to
+ // have optimizations that don't apply to indexOf(char, int)
+ // which result in a >10% performance improvement in some
+ // NamePatternConverterBenchmark cases. This should be re-evaluated with
+ // future java releases.
+ int nextDot = input.indexOf(".", inputIndex);
Review comment:
Great find! I really appreciate your insight, @schlosna. I'll a/b test on jdk16 to see how performance is impacted. If the character search is much better, it might be worthwhile to use that as it should stand the test of time, especially with jdk17 right around the corner.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] carterkozak commented on a change in pull request #555: Improve worst-case NameAbbreviator performance
Posted by GitBox <gi...@apache.org>.
carterkozak commented on a change in pull request #555:
URL: https://github.com/apache/logging-log4j2/pull/555#discussion_r682083949
##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/NameAbbreviator.java
##########
@@ -285,40 +285,40 @@ public PatternAbbreviatorFragment(
/**
* Abbreviate element of name.
*
- * @param buf buffer to receive element.
- * @param startPos starting index of name element.
- * @return starting index of next element.
+ * @param input input string which is being written to the output {@code buf}.
+ * @param inputIndex starting index of name element in the {@code input} string.
+ * @param buf buffer to receive element.
+ * @return starting index of next element.
*/
- public int abbreviate(final StringBuilder buf, final int startPos) {
- final int start = (startPos < 0) ? 0 : startPos;
- final int max = buf.length();
- int nextDot = -1;
- for (int i = start; i < max; i++) {
- if (buf.charAt(i) == '.') {
- nextDot = i;
- break;
- }
+ int abbreviate(final String input, final int inputIndex, final StringBuilder buf) {
+ // ckozak: indexOf with a string is intentional. indexOf(String, int) appears to
+ // have optimizations that don't apply to indexOf(char, int)
+ // which result in a >10% performance improvement in some
+ // NamePatternConverterBenchmark cases. This should be re-evaluated with
+ // future java releases.
+ int nextDot = input.indexOf(".", inputIndex);
Review comment:
Agreed. I verified on linux x86_64 with openjdk 11 and 15, but it's likely performance is different elsewhere (although not clear in which direction it differs).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] carterkozak commented on a change in pull request #555: Improve worst-case NameAbbreviator performance
Posted by GitBox <gi...@apache.org>.
carterkozak commented on a change in pull request #555:
URL: https://github.com/apache/logging-log4j2/pull/555#discussion_r682086487
##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/NameAbbreviator.java
##########
@@ -285,40 +285,40 @@ public PatternAbbreviatorFragment(
/**
* Abbreviate element of name.
*
- * @param buf buffer to receive element.
- * @param startPos starting index of name element.
- * @return starting index of next element.
+ * @param input input string which is being written to the output {@code buf}.
+ * @param inputIndex starting index of name element in the {@code input} string.
+ * @param buf buffer to receive element.
+ * @return starting index of next element.
*/
- public int abbreviate(final StringBuilder buf, final int startPos) {
- final int start = (startPos < 0) ? 0 : startPos;
- final int max = buf.length();
- int nextDot = -1;
- for (int i = start; i < max; i++) {
- if (buf.charAt(i) == '.') {
- nextDot = i;
- break;
- }
+ int abbreviate(final String input, final int inputIndex, final StringBuilder buf) {
+ // ckozak: indexOf with a string is intentional. indexOf(String, int) appears to
+ // have optimizations that don't apply to indexOf(char, int)
+ // which result in a >10% performance improvement in some
+ // NamePatternConverterBenchmark cases. This should be re-evaluated with
+ // future java releases.
+ int nextDot = input.indexOf(".", inputIndex);
+ if (nextDot < 0) {
+ buf.append(input, inputIndex, input.length());
+ return nextDot;
}
- if (nextDot != -1) {
- if (nextDot - startPos > charCount) {
- buf.delete(startPos + charCount, nextDot);
Review comment:
This line as well as the `buf.insert` on 308 are potentially costly because they shift the entire buffer beyond the deleted/inserted position. Small buffers are relatively inexpensive because everything fits nicely in cache, but larger values will have non-obvious performance problems.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] schlosna commented on a change in pull request #555: Improve worst-case NameAbbreviator performance
Posted by GitBox <gi...@apache.org>.
schlosna commented on a change in pull request #555:
URL: https://github.com/apache/logging-log4j2/pull/555#discussion_r683117411
##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/NameAbbreviator.java
##########
@@ -285,40 +285,40 @@ public PatternAbbreviatorFragment(
/**
* Abbreviate element of name.
*
- * @param buf buffer to receive element.
- * @param startPos starting index of name element.
- * @return starting index of next element.
+ * @param input input string which is being written to the output {@code buf}.
+ * @param inputIndex starting index of name element in the {@code input} string.
+ * @param buf buffer to receive element.
+ * @return starting index of next element.
*/
- public int abbreviate(final StringBuilder buf, final int startPos) {
- final int start = (startPos < 0) ? 0 : startPos;
- final int max = buf.length();
- int nextDot = -1;
- for (int i = start; i < max; i++) {
- if (buf.charAt(i) == '.') {
- nextDot = i;
- break;
- }
+ int abbreviate(final String input, final int inputIndex, final StringBuilder buf) {
+ // ckozak: indexOf with a string is intentional. indexOf(String, int) appears to
+ // have optimizations that don't apply to indexOf(char, int)
+ // which result in a >10% performance improvement in some
+ // NamePatternConverterBenchmark cases. This should be re-evaluated with
+ // future java releases.
+ int nextDot = input.indexOf(".", inputIndex);
Review comment:
Likely due to difference with intrinsic for `indexOf` with `String`, while the `char` version is not intrinsified until OpenJDK 16+, see https://bugs.openjdk.java.net/browse/JDK-8173585 , https://github.com/openjdk/jdk/commit/f71e8a61 and https://mail.openjdk.java.net/pipermail/jdk9-dev/2017-January/005539.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] rgoers commented on pull request #555: Improve worst-case NameAbbreviator performance
Posted by GitBox <gi...@apache.org>.
rgoers commented on pull request #555:
URL: https://github.com/apache/logging-log4j2/pull/555#issuecomment-969093561
I am unclear where things stand with this. I assume if you had wanted it incorporated you would have pushed it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] garydgregory commented on a change in pull request #555: Improve worst-case NameAbbreviator performance
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #555:
URL: https://github.com/apache/logging-log4j2/pull/555#discussion_r682082299
##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/NameAbbreviator.java
##########
@@ -285,40 +285,40 @@ public PatternAbbreviatorFragment(
/**
* Abbreviate element of name.
*
- * @param buf buffer to receive element.
- * @param startPos starting index of name element.
- * @return starting index of next element.
+ * @param input input string which is being written to the output {@code buf}.
+ * @param inputIndex starting index of name element in the {@code input} string.
+ * @param buf buffer to receive element.
+ * @return starting index of next element.
*/
- public int abbreviate(final StringBuilder buf, final int startPos) {
- final int start = (startPos < 0) ? 0 : startPos;
- final int max = buf.length();
- int nextDot = -1;
- for (int i = start; i < max; i++) {
- if (buf.charAt(i) == '.') {
- nextDot = i;
- break;
- }
+ int abbreviate(final String input, final int inputIndex, final StringBuilder buf) {
+ // ckozak: indexOf with a string is intentional. indexOf(String, int) appears to
+ // have optimizations that don't apply to indexOf(char, int)
+ // which result in a >10% performance improvement in some
+ // NamePatternConverterBenchmark cases. This should be re-evaluated with
+ // future java releases.
+ int nextDot = input.indexOf(".", inputIndex);
Review comment:
I've not looked at this but the scary part to me is that the performance might vary from JRE vendor to JRE vendor and from version to version.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] carterkozak commented on pull request #555: Improve worst-case NameAbbreviator performance
Posted by GitBox <gi...@apache.org>.
carterkozak commented on pull request #555:
URL: https://github.com/apache/logging-log4j2/pull/555#issuecomment-893543113
## Azul Zulu JDK 16.0.2, OpenJDK 64-Bit Server VM, 16.0.2+7
### This PR (as is, using `indexOf(String, int)`)
_1_ (run numbers labeled to account for thermal/scaling issues on my system)
```
Benchmark (className) Mode Cnt Score Error Units
NamePatternConverterBenchmark.benchNamePatternConverter org.bogus.hokus.pokus.org.bogus.hokus.pokus.org.bogus.hokus.pokus.RetroEncabulatorFactorySingleton avgt 4 195.817 ± 7.506 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter org.bogus.hokus.pokus.Clazz1 avgt 4 74.693 ± 0.583 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter com.bogus.hokus.pokus.Clazz2 avgt 4 74.509 ± 2.908 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter edu.bogus.hokus.pokus.a.Clazz3 avgt 4 90.755 ± 0.650 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter de.bogus.hokus.b.Clazz4 avgt 4 78.861 ± 0.699 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter jp.bogus.c.Clazz5 avgt 4 66.118 ± 2.626 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter cn.d.Clazz6 avgt 4 49.765 ± 5.760 ns/op
```
_4_
```
Benchmark (className) Mode Cnt Score Error Units
NamePatternConverterBenchmark.benchNamePatternConverter org.bogus.hokus.pokus.org.bogus.hokus.pokus.org.bogus.hokus.pokus.RetroEncabulatorFactorySingleton avgt 4 195.953 ± 13.193 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter org.bogus.hokus.pokus.Clazz1 avgt 4 74.750 ± 2.435 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter com.bogus.hokus.pokus.Clazz2 avgt 4 80.890 ± 29.628 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter edu.bogus.hokus.pokus.a.Clazz3 avgt 4 92.087 ± 3.809 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter de.bogus.hokus.b.Clazz4 avgt 4 79.318 ± 3.140 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter jp.bogus.c.Clazz5 avgt 4 62.846 ± 1.945 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter cn.d.Clazz6 avgt 4 48.112 ± 4.871 ns/op
```
### This PR, but using `indexOf(char, int)`
_2_
```
Benchmark (className) Mode Cnt Score Error Units
NamePatternConverterBenchmark.benchNamePatternConverter org.bogus.hokus.pokus.org.bogus.hokus.pokus.org.bogus.hokus.pokus.RetroEncabulatorFactorySingleton avgt 4 179.839 ± 8.086 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter org.bogus.hokus.pokus.Clazz1 avgt 4 75.254 ± 11.107 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter com.bogus.hokus.pokus.Clazz2 avgt 4 74.735 ± 3.403 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter edu.bogus.hokus.pokus.a.Clazz3 avgt 4 85.463 ± 1.292 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter de.bogus.hokus.b.Clazz4 avgt 4 71.888 ± 3.801 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter jp.bogus.c.Clazz5 avgt 4 59.703 ± 14.708 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter cn.d.Clazz6 avgt 4 44.177 ± 1.026 ns/op
```
_5_
```
Benchmark (className) Mode Cnt Score Error Units
NamePatternConverterBenchmark.benchNamePatternConverter org.bogus.hokus.pokus.org.bogus.hokus.pokus.org.bogus.hokus.pokus.RetroEncabulatorFactorySingleton avgt 4 178.932 ± 23.631 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter org.bogus.hokus.pokus.Clazz1 avgt 4 70.855 ± 1.261 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter com.bogus.hokus.pokus.Clazz2 avgt 4 71.347 ± 3.361 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter edu.bogus.hokus.pokus.a.Clazz3 avgt 4 82.970 ± 0.842 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter de.bogus.hokus.b.Clazz4 avgt 4 71.882 ± 3.248 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter jp.bogus.c.Clazz5 avgt 4 58.917 ± 2.620 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter cn.d.Clazz6 avgt 4 42.353 ± 0.672 ns/op
```
### Original implementation from `release-2.x`
_3_
```
Benchmark (className) Mode Cnt Score Error Units
NamePatternConverterBenchmark.benchNamePatternConverter org.bogus.hokus.pokus.org.bogus.hokus.pokus.org.bogus.hokus.pokus.RetroEncabulatorFactorySingleton avgt 4 201.906 ± 2.327 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter org.bogus.hokus.pokus.Clazz1 avgt 4 73.014 ± 3.037 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter com.bogus.hokus.pokus.Clazz2 avgt 4 74.045 ± 2.914 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter edu.bogus.hokus.pokus.a.Clazz3 avgt 4 89.466 ± 1.284 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter de.bogus.hokus.b.Clazz4 avgt 4 69.703 ± 2.335 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter jp.bogus.c.Clazz5 avgt 4 53.735 ± 0.696 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter cn.d.Clazz6 avgt 4 39.654 ± 0.318 ns/op
```
_6_
```
Benchmark (className) Mode Cnt Score Error Units
NamePatternConverterBenchmark.benchNamePatternConverter org.bogus.hokus.pokus.org.bogus.hokus.pokus.org.bogus.hokus.pokus.RetroEncabulatorFactorySingleton avgt 4 209.823 ± 13.330 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter org.bogus.hokus.pokus.Clazz1 avgt 4 73.479 ± 4.820 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter com.bogus.hokus.pokus.Clazz2 avgt 4 77.332 ± 7.192 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter edu.bogus.hokus.pokus.a.Clazz3 avgt 4 90.165 ± 4.701 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter de.bogus.hokus.b.Clazz4 avgt 4 70.995 ± 2.363 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter jp.bogus.c.Clazz5 avgt 4 53.924 ± 0.488 ns/op
NamePatternConverterBenchmark.benchNamePatternConverter cn.d.Clazz6 avgt 4 39.785 ± 0.219 ns/op
```
## Conclusion
@schlosna you are correct, jdk16 shows substantially better performance using `indexOf(char,int)`, the benchmark results align more closely with my original expectations as illustrated by the improvements abbreviating the longer `RetroEncabulatorFactorySingleton` class. The original implementation performs best on very short class names, but I think those are fast enough that it doesn't make much difference either way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] carterkozak commented on a change in pull request #555: Improve worst-case NameAbbreviator performance
Posted by GitBox <gi...@apache.org>.
carterkozak commented on a change in pull request #555:
URL: https://github.com/apache/logging-log4j2/pull/555#discussion_r682075572
##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/NameAbbreviator.java
##########
@@ -285,40 +285,40 @@ public PatternAbbreviatorFragment(
/**
* Abbreviate element of name.
*
- * @param buf buffer to receive element.
- * @param startPos starting index of name element.
- * @return starting index of next element.
+ * @param input input string which is being written to the output {@code buf}.
+ * @param inputIndex starting index of name element in the {@code input} string.
+ * @param buf buffer to receive element.
+ * @return starting index of next element.
*/
- public int abbreviate(final StringBuilder buf, final int startPos) {
- final int start = (startPos < 0) ? 0 : startPos;
- final int max = buf.length();
- int nextDot = -1;
- for (int i = start; i < max; i++) {
- if (buf.charAt(i) == '.') {
- nextDot = i;
- break;
- }
+ int abbreviate(final String input, final int inputIndex, final StringBuilder buf) {
+ // ckozak: indexOf with a string is intentional. indexOf(String, int) appears to
+ // have optimizations that don't apply to indexOf(char, int)
+ // which result in a >10% performance improvement in some
+ // NamePatternConverterBenchmark cases. This should be re-evaluated with
+ // future java releases.
+ int nextDot = input.indexOf(".", inputIndex);
Review comment:
This performance change was _wild_:
```diff
- int nextDot = input.indexOf('.', inputIndex);
+ int nextDot = input.indexOf(".", inputIndex);
```
@garydgregory Any chance you have ideas that might explain this? It threw me for a loop because the character-based indexOf underperforms compared to the old implementation, while the string-based indexOf has an advantage and much lower variance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org