You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2021/02/27 04:51:03 UTC
[GitHub] [incubator-dolphinscheduler] zhuangchong opened a new pull request #4896: [Improvement][Common] Optimize HashMap Initial Capacity
zhuangchong opened a new pull request #4896:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4896
## What is the purpose of the pull request
Optimize HashMap Initial Capacity
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-dolphinscheduler] CalvinKirs commented on a change in pull request #4896: [Improvement][Common] Optimize HashMap Initial Capacity
Posted by GitBox <gi...@apache.org>.
CalvinKirs commented on a change in pull request #4896:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4896#discussion_r584122148
##########
File path: dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/CollectionUtils.java
##########
@@ -103,7 +109,8 @@ public static boolean isEmpty(Collection coll) {
return emptyMap;
}
String[] strings = str.split(separator);
- Map<String, String> map = new HashMap<>(strings.length);
+ int initialCapacity = (int)(strings.length / DEFAULT_LOAD_FACTOR) + 1;
+ Map<String, String> map = new HashMap<>(initialCapacity);
Review comment:
you are right~
##########
File path: dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/CollectionUtils.java
##########
@@ -103,7 +109,8 @@ public static boolean isEmpty(Collection coll) {
return emptyMap;
}
String[] strings = str.split(separator);
- Map<String, String> map = new HashMap<>(strings.length);
+ int initialCapacity = (int)(strings.length / DEFAULT_LOAD_FACTOR) + 1;
+ Map<String, String> map = new HashMap<>(initialCapacity);
Review comment:
you are right~
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-dolphinscheduler] CalvinKirs merged pull request #4896: [Improvement][Common] Optimize HashMap Initial Capacity
Posted by GitBox <gi...@apache.org>.
CalvinKirs merged pull request #4896:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4896
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-dolphinscheduler] codecov-io commented on pull request #4896: [Improvement][Common] Optimize HashMap Initial Capacity
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #4896:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4896#issuecomment-787006152
# [Codecov](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4896?src=pr&el=h1) Report
> Merging [#4896](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4896?src=pr&el=desc) (a1693c0) into [dev](https://codecov.io/gh/apache/incubator-dolphinscheduler/commit/b8788f007b1617605e05d70c0ff33c4aa7318c4b?el=desc) (b8788f0) will **decrease** coverage by `0.16%`.
> The diff coverage is `80.00%`.
[](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4896?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## dev #4896 +/- ##
============================================
- Coverage 47.07% 46.90% -0.17%
+ Complexity 3639 3625 -14
============================================
Files 567 567
Lines 23811 23812 +1
Branches 2742 2742
============================================
- Hits 11209 11170 -39
- Misses 11588 11632 +44
+ Partials 1014 1010 -4
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4896?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...lphinscheduler/service/quartz/QuartzExecutors.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4896/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3NlcnZpY2UvcXVhcnR6L1F1YXJ0ekV4ZWN1dG9ycy5qYXZh) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
| [...olphinscheduler/api/controller/BaseController.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4896/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvQmFzZUNvbnRyb2xsZXIuamF2YQ==) | `53.75% <100.00%> (ø)` | `10.00 <1.00> (ø)` | |
| [...dolphinscheduler/common/utils/CollectionUtils.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4896/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL3V0aWxzL0NvbGxlY3Rpb25VdGlscy5qYXZh) | `92.64% <100.00%> (+0.10%)` | `30.00 <0.00> (ø)` | |
| [.../server/master/runner/DependentTaskExecThread.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4896/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9ydW5uZXIvRGVwZW5kZW50VGFza0V4ZWNUaHJlYWQuamF2YQ==) | `51.57% <0.00%> (-16.85%)` | `10.00% <0.00%> (-5.00%)` | |
| [...ver/master/consumer/TaskPriorityQueueConsumer.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4896/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9jb25zdW1lci9UYXNrUHJpb3JpdHlRdWV1ZUNvbnN1bWVyLmphdmE=) | `43.62% <0.00%> (-9.40%)` | `16.00% <0.00%> (-4.00%)` | |
| [...olphinscheduler/server/utils/DependentExecute.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4896/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL3V0aWxzL0RlcGVuZGVudEV4ZWN1dGUuamF2YQ==) | `61.84% <0.00%> (-9.22%)` | `17.00% <0.00%> (-4.00%)` | |
| [...olphinscheduler/plugin/alert/email/ExcelUtils.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4896/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1hbGVydC1wbHVnaW4vZG9scGhpbnNjaGVkdWxlci1hbGVydC1lbWFpbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZG9scGhpbnNjaGVkdWxlci9wbHVnaW4vYWxlcnQvZW1haWwvRXhjZWxVdGlscy5qYXZh) | `78.18% <0.00%> (-3.64%)` | `8.00% <0.00%> (-1.00%)` | |
| [...r/server/worker/processor/TaskCallbackService.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4896/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL3dvcmtlci9wcm9jZXNzb3IvVGFza0NhbGxiYWNrU2VydmljZS5qYXZh) | `38.09% <0.00%> (-3.18%)` | `8.00% <0.00%> (ø%)` | |
| [...inscheduler/server/master/config/MasterConfig.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4896/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9jb25maWcvTWFzdGVyQ29uZmlnLmphdmE=) | `42.42% <0.00%> (-3.04%)` | `11.00% <0.00%> (-1.00%)` | |
| [...inscheduler/service/zk/CuratorZookeeperClient.java](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4896/diff?src=pr&el=tree#diff-ZG9scGhpbnNjaGVkdWxlci1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kb2xwaGluc2NoZWR1bGVyL3NlcnZpY2UvemsvQ3VyYXRvclpvb2tlZXBlckNsaWVudC5qYXZh) | `65.85% <0.00%> (+4.87%)` | `8.00% <0.00%> (+1.00%)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4896?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4896?src=pr&el=footer). Last update [b8788f0...a1693c0](https://codecov.io/gh/apache/incubator-dolphinscheduler/pull/4896?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-dolphinscheduler] CalvinKirs commented on a change in pull request #4896: [Improvement][Common] Optimize HashMap Initial Capacity
Posted by GitBox <gi...@apache.org>.
CalvinKirs commented on a change in pull request #4896:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4896#discussion_r584050411
##########
File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/BaseController.java
##########
@@ -48,7 +48,7 @@
* @return check result code
*/
public Map<String, Object> checkPageParams(int pageNo, int pageSize) {
- Map<String, Object> result = new HashMap<>(2);
+ Map<String, Object> result = new HashMap<>(4);
Review comment:
In my opinion, this doesn't actually make much sense. No matter if the initial capacity is 2 or 4
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-dolphinscheduler] sonarcloud[bot] commented on pull request #4896: [Improvement][Common] Optimize HashMap Initial Capacity
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #4896:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4896#issuecomment-787006676
Kudos, SonarCloud Quality Gate passed!
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4896&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4896&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4896&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4896&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4896&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4896&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=4896&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=4896&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=4896&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4896&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4896&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=4896&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60.png' alt='80.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4896&metric=new_coverage&view=list) [80.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4896&metric=new_coverage&view=list)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4896&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=4896&metric=new_duplicated_lines_density&view=list)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-dolphinscheduler] zhuangchong commented on a change in pull request #4896: [Improvement][Common] Optimize HashMap Initial Capacity
Posted by GitBox <gi...@apache.org>.
zhuangchong commented on a change in pull request #4896:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4896#discussion_r584098814
##########
File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/BaseController.java
##########
@@ -48,7 +48,7 @@
* @return check result code
*/
public Map<String, Object> checkPageParams(int pageNo, int pageSize) {
- Map<String, Object> result = new HashMap<>(2);
+ Map<String, Object> result = new HashMap<>(4);
Review comment:
There is a difference. When the storage is 2 and the capacity is initialized to 2, automatic scaling of the HashMap is triggered
The HashMap load factor is 0.75 by default, the initialization capacity is 2, and the HashMap is automatically expanded when put the second element
I feel that can use the default capacity, or can specify the initial capacity, but can't specify an initial value that will trigger HashMap expansion in scenarios where storage is small
---
是有区别的，当存储为2并初始化容量为2会触发hashmap的自动扩容
hashmap负载因子是默认0.75,初始化容量为2，当put第二个元素的时候会触发hashmap自动扩容
我感觉可以使用默认容量，也可以指定初始容量，但是不能指定一个会触发hashmap扩容并在存储很小的场景下的一个初始值
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-dolphinscheduler] CalvinKirs commented on a change in pull request #4896: [Improvement][Common] Optimize HashMap Initial Capacity
Posted by GitBox <gi...@apache.org>.
CalvinKirs commented on a change in pull request #4896:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4896#discussion_r584121225
##########
File path: dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/BaseController.java
##########
@@ -48,7 +48,7 @@
* @return check result code
*/
public Map<String, Object> checkPageParams(int pageNo, int pageSize) {
- Map<String, Object> result = new HashMap<>(2);
+ Map<String, Object> result = new HashMap<>(4);
Review comment:
but..hashmap‘s min size is 16.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-dolphinscheduler] zhuangchong commented on a change in pull request #4896: [Improvement][Common] Optimize HashMap Initial Capacity
Posted by GitBox <gi...@apache.org>.
zhuangchong commented on a change in pull request #4896:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4896#discussion_r584099105
##########
File path: dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/CollectionUtils.java
##########
@@ -103,7 +109,8 @@ public static boolean isEmpty(Collection coll) {
return emptyMap;
}
String[] strings = str.split(separator);
- Map<String, String> map = new HashMap<>(strings.length);
+ int initialCapacity = (int)(strings.length / DEFAULT_LOAD_FACTOR) + 1;
+ Map<String, String> map = new HashMap<>(initialCapacity);
Review comment:
This change is to reduce the automatic scaling of the HashMap to get the maximum initial capacity based on the specified storage capacity and load factor.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-dolphinscheduler] CalvinKirs commented on a change in pull request #4896: [Improvement][Common] Optimize HashMap Initial Capacity
Posted by GitBox <gi...@apache.org>.
CalvinKirs commented on a change in pull request #4896:
URL: https://github.com/apache/incubator-dolphinscheduler/pull/4896#discussion_r584050628
##########
File path: dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/CollectionUtils.java
##########
@@ -103,7 +109,8 @@ public static boolean isEmpty(Collection coll) {
return emptyMap;
}
String[] strings = str.split(separator);
- Map<String, String> map = new HashMap<>(strings.length);
+ int initialCapacity = (int)(strings.length / DEFAULT_LOAD_FACTOR) + 1;
+ Map<String, String> map = new HashMap<>(initialCapacity);
Review comment:
keep old-style maybe better
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org