[ISSUE #5507]Improve the speed of AttributeParser#parseToMap parsing #5508
Conversation
caigy
left a comment
caigy
left a comment
There was a problem hiding this comment.
Although those variables should be constant, are there any test results to show the exact effect in improving the speed of parsing?
| public static final String COMMA = ","; | ||
|
|
||
| public static final String EQUAL_SIGN = "="; | ||
|
|
||
| public static final String PLUS_SIGN = "+"; | ||
|
|
||
| private static final String MINUS_SIGN = "-"; |
There was a problem hiding this comment.
Pls use more descriptive names, like 'ATTR_ARRAY_SEPARATOR'.
There was a problem hiding this comment.
@caigy test result on related issue
Although those variables should be constant, are there any test results to show the exact effect in improving the speed of parsing?
split two times
@BenchmarkMode(Mode.Throughput)
@Warmup(iterations = 3, time = 1)
@Measurement(iterations = 5, time = 5)
@Threads(1)
@Fork(1)
@State(value = Scope.Benchmark)
@OutputTimeUnit(TimeUnit.SECONDS)
public class SplitBenchmark {
private String bbb = "1111,2222";
@Benchmark
public void splitTwo() {
String s = bbb.split(",")[0];
String s1 = bbb.split(",")[1];
}
@Benchmark
public void splitOne() {
String[] split = bbb.split(",");
String s = split[0];
String s1 = split[1];
}
public static void main(String[] args) throws RunnerException {
Options opt = new OptionsBuilder()
.include(SplitBenchmark.class.getSimpleName())
.result("result.json")
.resultFormat(ResultFormatType.JSON).build();
new Runner(opt).run();
}
}benchmark result:
those variables should be constant is polish code
There was a problem hiding this comment.
Pls use more descriptive names, like 'ATTR_ARRAY_SEPARATOR'.
I will do, thanks for you suggest
mxsm
dismissed stale reviews from lizhanhui and tsunghanjacktsai
via
1b8a261
Codecov Report
@@ Coverage Diff @@
## develop #5508 +/- ##
=============================================
- Coverage 42.92% 42.72% -0.20%
+ Complexity 8069 8023 -46
=============================================
Files 1029 1029
Lines 72662 72672 +10
Branches 9604 9604
=============================================
- Hits 31191 31052 -139
- Misses 37527 37683 +156
+ Partials 3944 3937 -7
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| public static final String ATTR_ARRAY_SEPARATOR_COMMA = ","; | ||
|
|
||
| public static final String ATTR_ARRAY_SEPARATOR_EQUAL_SIGN = "="; | ||
|
|
||
| public static final String ATTR_ARRAY_SEPARATOR_PLUS_SIGN = "+"; | ||
|
|
||
| private static final String ATTR_ARRAY_SEPARATOR_MINUS_SIGN = "-"; |
There was a problem hiding this comment.
In fact not all of these constants are separators, pls give them names showing the exact purpose of them, eg. : What's the meaning of 'plus' or 'minus' in the context of attribute parsing?
BTW, I'd prefer making these constant fields as private, for they are not referenced outside AttributeParse.
There was a problem hiding this comment.
In fact not all of these constants are separators, pls give them names showing the exact purpose of them, eg. : What's the meaning of 'plus' or 'minus' in the context of attribute parsing? BTW, I'd prefer making these constant fields as
private, for they are not referenced outsideAttributeParse.
I see what you mean, thanks for you suggest, I will polish this code again.
|
@lizhanhui PTAL~ |
7 participants
Make sure set the target branch to
developWhat is the purpose of the change
close #5507
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.