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-unsubscr...@hc.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org
For additional commands, e-mail: dev-h...@hc.apache.org
