You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by GitBox <gi...@apache.org> on 2022/02/25 15:46:39 UTC
[GitHub] [httpcomponents-client] schlosna opened a new pull request #352: Optimize ExecSupport.getNextExchangeId()
schlosna opened a new pull request #352:
URL: https://github.com/apache/httpcomponents-client/pull/352
While analyzing some JFR profiles from a service utilizing a high volume of async HTTP client requests, I noticed one of the top memory allocations of `java.text.DecimalFormatSymbols` coming from `org.apache.hc.client5.http.impl.ExecSupport#getNextExchangeId`, notably due to use of `String#format`. Given that this seems to be on a hot code path, we can optimize this code path.
Backtrace:
```
sun.util.locale.provider.DecimalFormatSymbolsProviderImpl.getInstance(Locale)
java.text.DecimalFormatSymbols.getInstance(Locale)
java.util.Formatter.zero()
java.util.Formatter$FormatSpecifier.getZero(Locale)
java.util.Formatter$FormatSpecifier.localizedMagnitude(StringBuilder, CharSequence, int, Formatter$Flags, int, Locale)
java.util.Formatter$FormatSpecifier.print(long, Locale)
java.util.Formatter$FormatSpecifier.printInteger(Object, Locale)
java.util.Formatter$FormatSpecifier.print(Object, Locale)
java.util.Formatter.format(Locale, String, Object[])
java.util.Formatter.format(String, Object[])
java.lang.String.format(String, Object[])
org.apache.hc.client5.http.impl.ExecSupport.getNextExchangeId()
org.apache.hc.client5.http.impl.classic.InternalHttpClient.doExecute(HttpHost, ClassicHttpRequest, HttpContext)
org.apache.hc.client5.http.impl.classic.CloseableHttpClient.execute(ClassicHttpRequest, HttpContext)
org.apache.hc.client5.http.impl.classic.CloseableHttpClient.execute(ClassicHttpRequest)
```
With a hand rolled a method to perform the same functionality of `String.format("ex-%010d", long)` we see a roughly 20x speed-up in execution time and ~16x reduction in allocations on a JMH microbenchmark for just this function (see https://github.com/schlosna/httpclient-format-performance ). This PR currently only optimizes the hot exchange ID code path and not the endpoint ID code paths that use similar formatting (see https://github.com/apache/httpcomponents-client/pull/249 )
Benchmarks all run on 16 core Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz.
JDK 11.0.13, OpenJDK 64-Bit Server VM, 11.0.13+8-LTS
```
Benchmark Mode Cnt Score Error Units
FormatBenchmarks.stringFormat avgt 5 674.730 ± 7.821 ns/op
FormatBenchmarks.stringFormat:·gc.alloc.rate avgt 5 852.757 ± 9.814 MB/sec
FormatBenchmarks.stringFormat:·gc.alloc.rate.norm avgt 5 704.000 ± 0.001 B/op
FormatBenchmarks.stringFormat:·gc.churn.G1_Eden_Space avgt 5 862.955 ± 171.696 MB/sec
FormatBenchmarks.stringFormat:·gc.churn.G1_Eden_Space.norm avgt 5 712.471 ± 146.197 B/op
FormatBenchmarks.stringFormat:·gc.churn.G1_Old_Gen avgt 5 0.001 ± 0.002 MB/sec
FormatBenchmarks.stringFormat:·gc.churn.G1_Old_Gen.norm avgt 5 0.001 ± 0.002 B/op
FormatBenchmarks.stringFormat:·gc.count avgt 5 53.000 counts
FormatBenchmarks.stringFormat:·gc.time avgt 5 33.000 ms
FormatBenchmarks.zeroPad avgt 5 38.760 ± 1.082 ns/op
FormatBenchmarks.zeroPad:·gc.alloc.rate avgt 5 1855.680 ± 51.458 MB/sec
FormatBenchmarks.zeroPad:·gc.alloc.rate.norm avgt 5 88.000 ± 0.001 B/op
FormatBenchmarks.zeroPad:·gc.churn.G1_Eden_Space avgt 5 1859.501 ± 202.606 MB/sec
FormatBenchmarks.zeroPad:·gc.churn.G1_Eden_Space.norm avgt 5 88.186 ± 10.045 B/op
FormatBenchmarks.zeroPad:·gc.churn.G1_Old_Gen avgt 5 0.004 ± 0.002 MB/sec
FormatBenchmarks.zeroPad:·gc.churn.G1_Old_Gen.norm avgt 5 ≈ 10⁻⁴ B/op
FormatBenchmarks.zeroPad:·gc.count avgt 5 79.000 counts
FormatBenchmarks.zeroPad:·gc.time avgt 5 62.000 ms
```
JDK 15.0.5, OpenJDK 64-Bit Server VM, 15.0.5+3-MTS
```
Benchmark Mode Cnt Score Error Units
FormatBenchmarks.stringFormat avgt 5 1081.263 ± 50.140 ns/op
FormatBenchmarks.stringFormat:·gc.alloc.rate avgt 5 1094.601 ± 50.204 MB/sec
FormatBenchmarks.stringFormat:·gc.alloc.rate.norm avgt 5 1448.105 ± 0.005 B/op
FormatBenchmarks.stringFormat:·gc.churn.G1_Eden_Space avgt 5 1093.372 ± 0.707 MB/sec
FormatBenchmarks.stringFormat:·gc.churn.G1_Eden_Space.norm avgt 5 1446.645 ± 67.105 B/op
FormatBenchmarks.stringFormat:·gc.churn.G1_Survivor_Space avgt 5 0.004 ± 0.007 MB/sec
FormatBenchmarks.stringFormat:·gc.churn.G1_Survivor_Space.norm avgt 5 0.006 ± 0.009 B/op
FormatBenchmarks.stringFormat:·gc.count avgt 5 55.000 counts
FormatBenchmarks.stringFormat:·gc.time avgt 5 51.000 ms
FormatBenchmarks.zeroPad avgt 5 39.612 ± 1.489 ns/op
FormatBenchmarks.zeroPad:·gc.alloc.rate avgt 5 1815.969 ± 68.124 MB/sec
FormatBenchmarks.zeroPad:·gc.alloc.rate.norm avgt 5 88.006 ± 0.001 B/op
FormatBenchmarks.zeroPad:·gc.churn.G1_Eden_Space avgt 5 1829.120 ± 209.669 MB/sec
FormatBenchmarks.zeroPad:·gc.churn.G1_Eden_Space.norm avgt 5 88.641 ± 9.053 B/op
FormatBenchmarks.zeroPad:·gc.churn.G1_Survivor_Space avgt 5 0.008 ± 0.002 MB/sec
FormatBenchmarks.zeroPad:·gc.churn.G1_Survivor_Space.norm avgt 5 ≈ 10⁻³ B/op
FormatBenchmarks.zeroPad:·gc.count avgt 5 92.000 counts
FormatBenchmarks.zeroPad:·gc.time avgt 5 80.000 ms
```
JDK 17.0.1, OpenJDK 64-Bit Server VM, 17.0.1+12-LTS
```
Benchmark Mode Cnt Score Error Units
FormatBenchmarks.stringFormat avgt 5 871.173 ± 11.990 ns/op
FormatBenchmarks.stringFormat:·gc.alloc.rate avgt 5 1313.506 ± 18.053 MB/sec
FormatBenchmarks.stringFormat:·gc.alloc.rate.norm avgt 5 1400.111 ± 0.011 B/op
FormatBenchmarks.stringFormat:·gc.churn.G1_Eden_Space avgt 5 1314.209 ± 139.635 MB/sec
FormatBenchmarks.stringFormat:·gc.churn.G1_Eden_Space.norm avgt 5 1400.917 ± 157.487 B/op
FormatBenchmarks.stringFormat:·gc.churn.G1_Survivor_Space avgt 5 0.006 ± 0.008 MB/sec
FormatBenchmarks.stringFormat:·gc.churn.G1_Survivor_Space.norm avgt 5 0.006 ± 0.009 B/op
FormatBenchmarks.stringFormat:·gc.count avgt 5 81.000 counts
FormatBenchmarks.stringFormat:·gc.time avgt 5 53.000 ms
FormatBenchmarks.zeroPad avgt 5 41.057 ± 0.172 ns/op
FormatBenchmarks.zeroPad:·gc.alloc.rate avgt 5 1751.849 ± 7.370 MB/sec
FormatBenchmarks.zeroPad:·gc.alloc.rate.norm avgt 5 88.007 ± 0.001 B/op
FormatBenchmarks.zeroPad:·gc.churn.G1_Eden_Space avgt 5 1752.274 ± 171.164 MB/sec
FormatBenchmarks.zeroPad:·gc.churn.G1_Eden_Space.norm avgt 5 88.027 ± 8.268 B/op
FormatBenchmarks.zeroPad:·gc.churn.G1_Survivor_Space avgt 5 0.009 ± 0.005 MB/sec
FormatBenchmarks.zeroPad:·gc.churn.G1_Survivor_Space.norm avgt 5 ≈ 10⁻³ B/op
FormatBenchmarks.zeroPad:·gc.count avgt 5 108.000 counts
FormatBenchmarks.zeroPad:·gc.time avgt 5 69.000 ms
```
--
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: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] carterkozak commented on pull request #352: Optimize ExecSupport.getNextExchangeId()
Posted by GitBox <gi...@apache.org>.
carterkozak commented on pull request #352:
URL: https://github.com/apache/httpcomponents-client/pull/352#issuecomment-1051060746
hard-coding length 10 and using the switch would be nice, but I defer to your judgement
--
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: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] schlosna edited a comment on pull request #352: Optimize ExecSupport.getNextExchangeId()
Posted by GitBox <gi...@apache.org>.
schlosna edited a comment on pull request #352:
URL: https://github.com/apache/httpcomponents-client/pull/352#issuecomment-1051045250
@michael-o I pulled out a `PrefixedIncrementingId` class to enable reuse across `Exchange`, `PoolingHttpClientConnectionManager`, `PoolingAsyncClientConnectionManager`.
@carterkozak defer to you on which implementation path you'd prefer given https://github.com/schlosna/httpclient-format-performance/pull/1
--
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: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] ok2c commented on pull request #352: Optimize ExecSupport.getNextExchangeId()
Posted by GitBox <gi...@apache.org>.
ok2c commented on pull request #352:
URL: https://github.com/apache/httpcomponents-client/pull/352#issuecomment-1052081135
Thank you so much, @schlosna. I also ported your changes to the 5.1.x branch.
--
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: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] schlosna commented on a change in pull request #352: Optimize ExecSupport.getNextExchangeId()
Posted by GitBox <gi...@apache.org>.
schlosna commented on a change in pull request #352:
URL: https://github.com/apache/httpcomponents-client/pull/352#discussion_r814961372
##########
File path: httpclient5/src/main/java/org/apache/hc/client5/http/impl/ExecSupport.java
##########
@@ -45,7 +45,29 @@ public static long getNextExecNumber() {
}
public static String getNextExchangeId() {
- return String.format("ex-%010d", COUNT.incrementAndGet());
+ return createId(COUNT.incrementAndGet());
+ }
+
+ /**
+ * Create an exchange ID.
+ *
+ * Hand rolled equivalent to `String.format("ex-%010d", value)` optimized to reduce
+ * allocation and CPU overhead.
+ */
+ static String createId(long value) {
+ String longString = Long.toString(value);
+ return "ex-" + zeroPad(10 - longString.length()) + longString;
Review comment:
Copying over @carterkozak 's updated benchmarks from https://github.com/schlosna/httpclient-format-performance/pull/1
JDK17 results
```
Benchmark Mode Cnt Score Error Units
FormatBenchmarks.stringFormat avgt 5 431.935 ± 37.370 ns/op
FormatBenchmarks.zeroPadPrefixesArray avgt 5 38.261 ± 1.599 ns/op
FormatBenchmarks.zeroPadPrefixesSwitch avgt 5 26.841 ± 2.509 ns/op
FormatBenchmarks.zeroPadProposed avgt 5 39.199 ± 1.526 ns/op
```
JDK8 results
```
Benchmark Mode Cnt Score Error Units
FormatBenchmarks.stringFormat avgt 5 514.358 ± 61.744 ns/op
FormatBenchmarks.zeroPadPrefixesArray avgt 5 56.109 ± 2.994 ns/op
FormatBenchmarks.zeroPadPrefixesSwitch avgt 5 43.931 ± 6.315 ns/op
FormatBenchmarks.zeroPadProposed avgt 5 70.111 ± 5.690 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: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] schlosna commented on pull request #352: Optimize ExecSupport.getNextExchangeId()
Posted by GitBox <gi...@apache.org>.
schlosna commented on pull request #352:
URL: https://github.com/apache/httpcomponents-client/pull/352#issuecomment-1054304305
Excellent, thanks @ok2c !
--
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: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] carterkozak commented on a change in pull request #352: Optimize ExecSupport.getNextExchangeId()
Posted by GitBox <gi...@apache.org>.
carterkozak commented on a change in pull request #352:
URL: https://github.com/apache/httpcomponents-client/pull/352#discussion_r814890829
##########
File path: httpclient5/src/main/java/org/apache/hc/client5/http/impl/ExecSupport.java
##########
@@ -45,7 +45,29 @@ public static long getNextExecNumber() {
}
public static String getNextExchangeId() {
- return String.format("ex-%010d", COUNT.incrementAndGet());
+ return createId(COUNT.incrementAndGet());
+ }
+
+ /**
+ * Create an exchange ID.
+ *
+ * Hand rolled equivalent to `String.format("ex-%010d", value)` optimized to reduce
+ * allocation and CPU overhead.
+ */
+ static String createId(long value) {
+ String longString = Long.toString(value);
+ return "ex-" + zeroPad(10 - longString.length()) + longString;
Review comment:
Thoughts on pre-computing the ten `"ex-0..."` prefixes?
--
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: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] carterkozak commented on a change in pull request #352: Optimize ExecSupport.getNextExchangeId()
Posted by GitBox <gi...@apache.org>.
carterkozak commented on a change in pull request #352:
URL: https://github.com/apache/httpcomponents-client/pull/352#discussion_r814919595
##########
File path: httpclient5/src/main/java/org/apache/hc/client5/http/impl/ExecSupport.java
##########
@@ -45,7 +45,29 @@ public static long getNextExecNumber() {
}
public static String getNextExchangeId() {
- return String.format("ex-%010d", COUNT.incrementAndGet());
+ return createId(COUNT.incrementAndGet());
+ }
+
+ /**
+ * Create an exchange ID.
+ *
+ * Hand rolled equivalent to `String.format("ex-%010d", value)` optimized to reduce
+ * allocation and CPU overhead.
+ */
+ static String createId(long value) {
+ String longString = Long.toString(value);
+ return "ex-" + zeroPad(10 - longString.length()) + longString;
Review comment:
The proposed implementation is far better than `String.format`, and I'm in favor of merging as is. Improvements from pre-computation will be insignificant by comparison.
That said, I think there are some small issues with the benchmark. It's using an AtomicLong counter, and incrementing for each benchmark. This helpfully matches the request ID generator, however in the benchmark we're running ~40ns operations for 45 seconds, beginning at 9,999,000,000 and incrementing roughly 1,125,000,000 times. I don't think the benchmark covers any cases where the precomputed values are used.
--
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: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] ok2c merged pull request #352: Optimize ExecSupport.getNextExchangeId()
Posted by GitBox <gi...@apache.org>.
ok2c merged pull request #352:
URL: https://github.com/apache/httpcomponents-client/pull/352
--
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: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] schlosna commented on pull request #352: Optimize ExecSupport.getNextExchangeId()
Posted by GitBox <gi...@apache.org>.
schlosna commented on pull request #352:
URL: https://github.com/apache/httpcomponents-client/pull/352#issuecomment-1051074768
Updated to use switch
--
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: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] ok2c commented on pull request #352: Optimize ExecSupport.getNextExchangeId()
Posted by GitBox <gi...@apache.org>.
ok2c commented on pull request #352:
URL: https://github.com/apache/httpcomponents-client/pull/352#issuecomment-1050990006
@schlosna Awesome! Could you please fix style-check violations, though?
--
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: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] schlosna commented on a change in pull request #352: Optimize ExecSupport.getNextExchangeId()
Posted by GitBox <gi...@apache.org>.
schlosna commented on a change in pull request #352:
URL: https://github.com/apache/httpcomponents-client/pull/352#discussion_r814902188
##########
File path: httpclient5/src/main/java/org/apache/hc/client5/http/impl/ExecSupport.java
##########
@@ -45,7 +45,29 @@ public static long getNextExecNumber() {
}
public static String getNextExchangeId() {
- return String.format("ex-%010d", COUNT.incrementAndGet());
+ return createId(COUNT.incrementAndGet());
+ }
+
+ /**
+ * Create an exchange ID.
+ *
+ * Hand rolled equivalent to `String.format("ex-%010d", value)` optimized to reduce
+ * allocation and CPU overhead.
+ */
+ static String createId(long value) {
+ String longString = Long.toString(value);
+ return "ex-" + zeroPad(10 - longString.length()) + longString;
Review comment:
Testing pregenerated prefixes via https://github.com/schlosna/httpclient-format-performance/commit/5c3d850ae997b58f73e5dc9170b335fa0bd0276a doesn't actually seem worth the extra complexity
JDK 17.0.1, OpenJDK 64-Bit Server VM, 17.0.1+12-LTS
```
Benchmark Mode Cnt Score Error Units
FormatBenchmarks.stringFormat avgt 5 872.268 ± 13.531 ns/op
FormatBenchmarks.stringFormat:·gc.alloc.rate avgt 5 1311.906 ± 20.298 MB/sec
FormatBenchmarks.stringFormat:·gc.alloc.rate.norm avgt 5 1400.112 ± 0.010 B/op
FormatBenchmarks.stringFormat:·gc.churn.G1_Eden_Space avgt 5 1314.251 ± 139.660 MB/sec
FormatBenchmarks.stringFormat:·gc.churn.G1_Eden_Space.norm avgt 5 1402.548 ± 134.966 B/op
FormatBenchmarks.stringFormat:·gc.churn.G1_Survivor_Space avgt 5 0.007 ± 0.003 MB/sec
FormatBenchmarks.stringFormat:·gc.churn.G1_Survivor_Space.norm avgt 5 0.007 ± 0.003 B/op
FormatBenchmarks.stringFormat:·gc.count avgt 5 81.000 counts
FormatBenchmarks.stringFormat:·gc.time avgt 5 60.000 ms
FormatBenchmarks.zeroPad avgt 5 42.288 ± 1.965 ns/op
FormatBenchmarks.zeroPad:·gc.alloc.rate avgt 5 1701.090 ± 79.519 MB/sec
FormatBenchmarks.zeroPad:·gc.alloc.rate.norm avgt 5 88.006 ± 0.001 B/op
FormatBenchmarks.zeroPad:·gc.churn.G1_Eden_Space avgt 5 1709.814 ± 171.192 MB/sec
FormatBenchmarks.zeroPad:·gc.churn.G1_Eden_Space.norm avgt 5 88.468 ± 9.798 B/op
FormatBenchmarks.zeroPad:·gc.churn.G1_Survivor_Space avgt 5 0.009 ± 0.005 MB/sec
FormatBenchmarks.zeroPad:·gc.churn.G1_Survivor_Space.norm avgt 5 ≈ 10⁻³ B/op
FormatBenchmarks.zeroPad:·gc.count avgt 5 86.000 counts
FormatBenchmarks.zeroPad:·gc.time avgt 5 73.000 ms
```
--
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: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] ok2c commented on a change in pull request #352: Optimize ExecSupport.getNextExchangeId()
Posted by GitBox <gi...@apache.org>.
ok2c commented on a change in pull request #352:
URL: https://github.com/apache/httpcomponents-client/pull/352#discussion_r814901897
##########
File path: httpclient5/src/main/java/org/apache/hc/client5/http/impl/ExecSupport.java
##########
@@ -45,7 +45,29 @@ public static long getNextExecNumber() {
}
public static String getNextExchangeId() {
- return String.format("ex-%010d", COUNT.incrementAndGet());
+ return createId(COUNT.incrementAndGet());
+ }
+
+ /**
+ * Create an exchange ID.
+ *
+ * Hand rolled equivalent to `String.format("ex-%010d", value)` optimized to reduce
+ * allocation and CPU overhead.
+ */
+ static String createId(long value) {
+ String longString = Long.toString(value);
+ return "ex-" + zeroPad(10 - longString.length()) + longString;
Review comment:
@carterkozak That would be clever.
--
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: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] michael-o commented on a change in pull request #352: Optimize ExecSupport.getNextExchangeId()
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #352:
URL: https://github.com/apache/httpcomponents-client/pull/352#discussion_r814892020
##########
File path: httpclient5/src/main/java/org/apache/hc/client5/http/impl/ExecSupport.java
##########
@@ -45,7 +45,29 @@ public static long getNextExecNumber() {
}
public static String getNextExchangeId() {
- return String.format("ex-%010d", COUNT.incrementAndGet());
+ return createId(COUNT.incrementAndGet());
+ }
+
+ /**
+ * Create an exchange ID.
+ *
+ * Hand rolled equivalent to `String.format("ex-%010d", value)` optimized to reduce
+ * allocation and CPU overhead.
+ */
+ static String createId(long value) {
+ String longString = Long.toString(value);
+ return "ex-" + zeroPad(10 - longString.length()) + longString;
Review comment:
@carterkozak how?
--
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: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] carterkozak commented on a change in pull request #352: Optimize ExecSupport.getNextExchangeId()
Posted by GitBox <gi...@apache.org>.
carterkozak commented on a change in pull request #352:
URL: https://github.com/apache/httpcomponents-client/pull/352#discussion_r814900201
##########
File path: httpclient5/src/main/java/org/apache/hc/client5/http/impl/ExecSupport.java
##########
@@ -45,7 +45,29 @@ public static long getNextExecNumber() {
}
public static String getNextExchangeId() {
- return String.format("ex-%010d", COUNT.incrementAndGet());
+ return createId(COUNT.incrementAndGet());
+ }
+
+ /**
+ * Create an exchange ID.
+ *
+ * Hand rolled equivalent to `String.format("ex-%010d", value)` optimized to reduce
+ * allocation and CPU overhead.
+ */
+ static String createId(long value) {
+ String longString = Long.toString(value);
+ return "ex-" + zeroPad(10 - longString.length()) + longString;
Review comment:
Either a switch:
```java
switch (longString.length()) { // It's too bad java.lang.Long.stringSize(long) is package-private
case 1:
return "ex-000000000" + longString;
/* etc... */
case 9:
return "ex-0" + longString;
default:
return "ex-" + longString;
}
```
Or an array of pre-computed values where we bounds-check and load a prefix based on the strlen of the value:
```java
private static final prefixes = new String[] {"ex-000000000", /* etc... */ ex-0"}
return len >= 10 ? "ex-" + longString : prefixes[len-1] + longString;
```
--
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: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] schlosna commented on pull request #352: Optimize ExecSupport.getNextExchangeId()
Posted by GitBox <gi...@apache.org>.
schlosna commented on pull request #352:
URL: https://github.com/apache/httpcomponents-client/pull/352#issuecomment-1051045250
@michael-o I pulled out an `IncrementingId` class to enable reuse across `Exchange`, `PoolingHttpClientConnectionManager`, `PoolingAsyncClientConnectionManager`.
@carterkozak defer to you on which implementation path you'd prefer given https://github.com/schlosna/httpclient-format-performance/pull/1
--
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: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org
[GitHub] [httpcomponents-client] carterkozak commented on a change in pull request #352: Optimize ExecSupport.getNextExchangeId()
Posted by GitBox <gi...@apache.org>.
carterkozak commented on a change in pull request #352:
URL: https://github.com/apache/httpcomponents-client/pull/352#discussion_r814900201
##########
File path: httpclient5/src/main/java/org/apache/hc/client5/http/impl/ExecSupport.java
##########
@@ -45,7 +45,29 @@ public static long getNextExecNumber() {
}
public static String getNextExchangeId() {
- return String.format("ex-%010d", COUNT.incrementAndGet());
+ return createId(COUNT.incrementAndGet());
+ }
+
+ /**
+ * Create an exchange ID.
+ *
+ * Hand rolled equivalent to `String.format("ex-%010d", value)` optimized to reduce
+ * allocation and CPU overhead.
+ */
+ static String createId(long value) {
+ String longString = Long.toString(value);
+ return "ex-" + zeroPad(10 - longString.length()) + longString;
Review comment:
Either a switch:
```java
switch (longString.length()) { // It's too bad java.lang.Long.stringSize(long) is package-private
case 1:
return "ex-000000000" + longString;
/* etc... */
case 9:
return "ex-0" + longString;
default:
return "ex-" + longString;
}
```
Or an array of pre-computed values where we bounds-check and load a prefix based on the strlen of the value:
```java
private static final prefixes = new String[] {"ex-000000000", /* etc... */ ex-0"}
return len > 10 ? "ex-" + longString : prefixes[len-1] + longString;
```
--
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: dev-unsubscribe@hc.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org