[ISSUE #5859]Optimize String#format in DefaultMQProducerImpl #5870
Conversation
| StringBuilder info = new StringBuilder(300); | ||
| info.append("Send [").append(times).append("] times, still failed, cost [") | ||
| .append(System.currentTimeMillis() - beginTimestampFirst).append("]ms, Topic: ").append(msg.getTopic()) | ||
| .append(", BrokersSent: ").append(Arrays.toString(brokersSent)) | ||
| .append(FAQUrl.suggestTodo(FAQUrl.SEND_MSG_FAILED)); |
There was a problem hiding this comment.
It seems that the String.format is more readable here
There was a problem hiding this comment.
It seems that the String.format is more readable here
@RongtongJin This optimization has been done from a performance perspective. The performance of String.format is relatively poor and triggers an array expansion, which can be referenced in issue #5804 . However, readability has been reduced.Performance enhancement should be considered as the first priority. Or Add a comment that enhances readability.
There was a problem hiding this comment.
IMO, these transitions are all in the exception scene,
it will lost a little performance when in the little scene,
so we can choose the readability when in this scene.
There was a problem hiding this comment.
Use MoreObjects.toStringHelper of guava or ToStringBuilder of commons-lang is more readable
Codecov Report
@@ Coverage Diff @@
## develop #5870 +/- ##
=============================================
- Coverage 42.48% 42.40% -0.09%
- Complexity 8248 8283 +35
=============================================
Files 1060 1066 +6
Lines 73473 73670 +197
Branches 9616 9629 +13
=============================================
+ Hits 31215 31239 +24
- Misses 38334 38476 +142
- Partials 3924 3955 +31
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Make sure set the target branch to
developWhat is the purpose of the change
close #5859
Brief changelog
Verifying this change
XXXX
Follow this checklist to help us incorporate your contribution quickly and easily. Notice,
it would be helpful if you could finish the following 5 checklist(the last one is not necessary)before request the community to review your PR.[ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.mvn -B clean apache-rat:check findbugs:findbugs checkstyle:checkstyleto make sure basic checks pass. Runmvn clean install -DskipITsto make sure unit-test pass. Runmvn clean test-compile failsafe:integration-testto make sure integration-test pass.