You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by GitBox <gi...@apache.org> on 2022/05/28 02:37:38 UTC
[GitHub] [rocketmq] dugenkui03 opened a new pull request, #4390: [ISSUE #4389] Optimize for MixAll#properties2Object
dugenkui03 opened a new pull request, #4390:
URL: https://github.com/apache/rocketmq/pull/4390
**Make sure set the target branch to `develop`**
## What is the purpose of the change
XXXXX
## Brief change log
Optimize for `MixAll#properties2Object`:
## Brief change log
1. Add debug log for exception;
2. Add processing logic for `byte`, `short` and `char`;
3. Reorder `if ... else`.
## 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`.
- [x] Make sure there is a [Github issue](https://github.com/apache/rocketmq/issues) filed for the change (usually before you start working on it). Trivial changes like typos do not require a Github issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
- [x] Format the pull request title like `[ISSUE #123] Fix UnknownException when host config not exist`. Each commit in the pull request should have a meaningful subject line and body.
- [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [x] Write necessary unit-test(over 80% coverage) to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add integration-test in [test module](https://github.com/apache/rocketmq/tree/master/test).
- [x] Run `mvn -B clean apache-rat:check findbugs:findbugs checkstyle:checkstyle` to make sure basic checks pass. Run `mvn clean install -DskipITs` to make sure unit-test pass. Run `mvn clean test-compile failsafe:integration-test` to make sure integration-test pass.
- [ ] If this contribution is large, please file an [Apache Individual Contributor License Agreement](http://www.apache.org/licenses/#clas).
--
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@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] HScarb commented on pull request #4390: [ISSUE #4389] Optimize for MixAll#properties2Object
Posted by GitBox <gi...@apache.org>.
HScarb commented on PR #4390:
URL: https://github.com/apache/rocketmq/pull/4390#issuecomment-1147096798
Is it necessary to add parsing of new types?
As many of the fields are `String`, will more `else if` branches cost more time to parse `String` fields?
--
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@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] dugenkui03 closed pull request #4390: [ISSUE #4389] Optimize for MixAll#properties2Object
Posted by "dugenkui03 (via GitHub)" <gi...@apache.org>.
dugenkui03 closed pull request #4390: [ISSUE #4389] Optimize for MixAll#properties2Object
URL: https://github.com/apache/rocketmq/pull/4390
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] codecov-commenter commented on pull request #4390: [ISSUE #4389] Optimize for MixAll#properties2Object
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #4390:
URL: https://github.com/apache/rocketmq/pull/4390#issuecomment-1140160120
# [Codecov](https://codecov.io/gh/apache/rocketmq/pull/4390?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#4390](https://codecov.io/gh/apache/rocketmq/pull/4390?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bc2974d) into [develop](https://codecov.io/gh/apache/rocketmq/commit/82a48be00a6d9def14abb871a40cb68cf8a041e9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (82a48be) will **decrease** coverage by `0.04%`.
> The diff coverage is `20.00%`.
```diff
@@ Coverage Diff @@
## develop #4390 +/- ##
=============================================
- Coverage 48.18% 48.14% -0.04%
- Complexity 5082 5084 +2
=============================================
Files 642 642
Lines 42770 42791 +21
Branches 5597 5601 +4
=============================================
- Hits 20609 20602 -7
- Misses 19655 19670 +15
- Partials 2506 2519 +13
```
| [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/4390?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...c/main/java/org/apache/rocketmq/common/MixAll.java](https://codecov.io/gh/apache/rocketmq/pull/4390/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vTWl4QWxsLmphdmE=) | `42.29% <20.00%> (-2.16%)` | :arrow_down: |
| [...va/org/apache/rocketmq/store/FlushDiskWatcher.java](https://codecov.io/gh/apache/rocketmq/pull/4390/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL0ZsdXNoRGlza1dhdGNoZXIuamF2YQ==) | `81.25% <0.00%> (-9.38%)` | :arrow_down: |
| [...org/apache/rocketmq/common/stats/StatsItemSet.java](https://codecov.io/gh/apache/rocketmq/pull/4390/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vc3RhdHMvU3RhdHNJdGVtU2V0LmphdmE=) | `44.77% <0.00%> (-5.98%)` | :arrow_down: |
| [...in/java/org/apache/rocketmq/test/util/MQAdmin.java](https://codecov.io/gh/apache/rocketmq/pull/4390/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-dGVzdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvdGVzdC91dGlsL01RQWRtaW4uamF2YQ==) | `38.88% <0.00%> (-5.56%)` | :arrow_down: |
| [...org/apache/rocketmq/store/ha/WaitNotifyObject.java](https://codecov.io/gh/apache/rocketmq/pull/4390/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL2hhL1dhaXROb3RpZnlPYmplY3QuamF2YQ==) | `66.07% <0.00%> (-5.36%)` | :arrow_down: |
| [...ketmq/common/protocol/body/ConsumerConnection.java](https://codecov.io/gh/apache/rocketmq/pull/4390/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcHJvdG9jb2wvYm9keS9Db25zdW1lckNvbm5lY3Rpb24uamF2YQ==) | `95.83% <0.00%> (-4.17%)` | :arrow_down: |
| [...ava/org/apache/rocketmq/filter/util/BitsArray.java](https://codecov.io/gh/apache/rocketmq/pull/4390/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZmlsdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9maWx0ZXIvdXRpbC9CaXRzQXJyYXkuamF2YQ==) | `59.82% <0.00%> (-2.57%)` | :arrow_down: |
| [...va/org/apache/rocketmq/logging/inner/Appender.java](https://codecov.io/gh/apache/rocketmq/pull/4390/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bG9nZ2luZy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvbG9nZ2luZy9pbm5lci9BcHBlbmRlci5qYXZh) | `34.83% <0.00%> (-2.25%)` | :arrow_down: |
| [...mq/client/impl/consumer/RebalanceLitePullImpl.java](https://codecov.io/gh/apache/rocketmq/pull/4390/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9jb25zdW1lci9SZWJhbGFuY2VMaXRlUHVsbEltcGwuamF2YQ==) | `72.05% <0.00%> (-1.48%)` | :arrow_down: |
| [...he/rocketmq/client/trace/AsyncTraceDispatcher.java](https://codecov.io/gh/apache/rocketmq/pull/4390/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvdHJhY2UvQXN5bmNUcmFjZURpc3BhdGNoZXIuamF2YQ==) | `80.97% <0.00%> (-1.47%)` | :arrow_down: |
| ... and [21 more](https://codecov.io/gh/apache/rocketmq/pull/4390/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/rocketmq/pull/4390?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/rocketmq/pull/4390?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [82a48be...bc2974d](https://codecov.io/gh/apache/rocketmq/pull/4390?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] MatrixHB commented on pull request #4390: [ISSUE #4389] Optimize for MixAll#properties2Object
Posted by GitBox <gi...@apache.org>.
MatrixHB commented on PR #4390:
URL: https://github.com/apache/rocketmq/pull/4390#issuecomment-1156064473
Related to [ISSUE #4389](https://github.com/apache/rocketmq/issues/4389)
--
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@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] coveralls commented on pull request #4390: [ISSUE #4389] Optimize for MixAll#properties2Object
Posted by GitBox <gi...@apache.org>.
coveralls commented on PR #4390:
URL: https://github.com/apache/rocketmq/pull/4390#issuecomment-1140161064
[](https://coveralls.io/builds/49533200)
Coverage decreased (-0.03%) to 52.217% when pulling **bc2974d3e3722068cf1093129f7ff33b653b2f78 on dugenkui03:optimize_for_properties2Object** into **af011b1e2d4395a5619ba9ffd27769d4d5ecdd19 on apache:develop**.
--
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@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] coveralls commented on pull request #4390: [ISSUE #4389] Optimize for MixAll#properties2Object
Posted by GitBox <gi...@apache.org>.
coveralls commented on PR #4390:
URL: https://github.com/apache/rocketmq/pull/4390#issuecomment-1140161063
[](https://coveralls.io/builds/49533200)
Coverage decreased (-0.03%) to 52.217% when pulling **bc2974d3e3722068cf1093129f7ff33b653b2f78 on dugenkui03:optimize_for_properties2Object** into **af011b1e2d4395a5619ba9ffd27769d4d5ecdd19 on apache:develop**.
--
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@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] xiaoyifang commented on a diff in pull request #4390: [ISSUE #4389] Optimize for MixAll#properties2Object
Posted by "xiaoyifang (via GitHub)" <gi...@apache.org>.
xiaoyifang commented on code in PR #4390:
URL: https://github.com/apache/rocketmq/pull/4390#discussion_r1133598267
##########
common/src/test/java/org/apache/rocketmq/common/UtilAllTest.java:
##########
@@ -60,11 +60,17 @@ public void testProperties2String() {
demoConfig.setDemoWidth(456);
demoConfig.setDemoName("TestDemo");
demoConfig.setDemoOK(true);
+ demoConfig.setDemoByte((byte) 78);
+ demoConfig.setDemoShort((short) 90);
+ demoConfig.setDemoChar('c');
Properties properties = MixAll.object2Properties(demoConfig);
assertThat(properties.getProperty("demoLength")).isEqualTo("123");
assertThat(properties.getProperty("demoWidth")).isEqualTo("456");
assertThat(properties.getProperty("demoOK")).isEqualTo("true");
Review Comment:
you have modified properties2Object, but test object2Properties method?
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org