Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

optimize: Local variable 'map' is redundant and check queue offer return value #3188

Merged
merged 20 commits into from
Nov 11, 2020

Conversation

everyhook1
Copy link
Contributor

Ⅰ. Describe what this PR did

https://github.com/seata/seata/blob/6c0c75939dedbf19c4a21a8ac7aa0fb98879a8fd/core/src/main/java/io/seata/core/rpc/netty/AbstractNettyRemotingClient.java#L156

// put message into basketMap
ConcurrentHashMap<String, BlockingQueue> map = basketMap;
BlockingQueue basket = map.get(serverAddress);
if (basket == null) {
map.putIfAbsent(serverAddress, new LinkedBlockingQueue<>());
basket = map.get(serverAddress);
}
basket.offer(rpcMessage);

1.remove redundant local variable 'map';
2.check the BlockingQueue offer return value;

Ⅱ. Does this pull request fix one issue?

fixes #3187

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@funky-eyes funky-eyes added theme: wiki-document Category issues or prs related to document. module/core core module and removed theme: wiki-document Category issues or prs related to document. labels Oct 13, 2020
if (basket == null) {
map.putIfAbsent(serverAddress, new LinkedBlockingQueue<>());
basket = map.get(serverAddress);
basketMap.putIfAbsent(serverAddress, new LinkedBlockingQueue<>());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

basketMap.computeIfAbsent(serverAddress, k -> new LinkedBlockingQueue<>());
I think it's better that way. what do you think?

liubin01 added 2 commits October 14, 2020 10:32
* origin/develop:
  optimize: Local variable 'map' is redundant and check queue offer return value apache#3187
@codecov-io
Copy link

codecov-io commented Oct 14, 2020

Codecov Report

Merging #3188 (2e57d05) into develop (43f5890) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #3188      +/-   ##
=============================================
- Coverage      51.03%   51.03%   -0.01%     
+ Complexity      3281     3246      -35     
=============================================
  Files            612      608       -4     
  Lines          20112    19975     -137     
  Branches        2517     2495      -22     
=============================================
- Hits           10265    10195      -70     
+ Misses          8820     8764      -56     
+ Partials        1027     1016      -11     
Impacted Files Coverage Δ Complexity Δ
...ta/core/rpc/netty/AbstractNettyRemotingClient.java 18.40% <0.00%> (-0.19%) 8.00 <0.00> (ø)
...o/seata/core/protocol/AbstractIdentifyRequest.java 50.00% <0.00%> (-20.00%) 3.00% <0.00%> (-2.00%)
.../rm/datasource/exec/BaseTransactionalExecutor.java 59.84% <0.00%> (-3.15%) 26.00% <0.00%> (-2.00%)
...zer/seata/protocol/AbstractResultMessageCodec.java 75.00% <0.00%> (-2.78%) 5.00% <0.00%> (ø%)
...c/main/java/io/seata/config/FileConfiguration.java 41.77% <0.00%> (-1.53%) 11.00% <0.00%> (ø%)
...n/java/io/seata/rm/datasource/DataSourceProxy.java 41.93% <0.00%> (-0.93%) 12.00% <0.00%> (ø%)
...ver/storage/db/session/DataBaseSessionManager.java 36.20% <0.00%> (-0.64%) 12.00% <0.00%> (ø%)
...a/io/seata/rm/datasource/xa/DataSourceProxyXA.java 84.00% <0.00%> (-0.62%) 8.00% <0.00%> (ø%)
...o/seata/server/coordinator/DefaultCoordinator.java 54.63% <0.00%> (-0.52%) 28.00% <0.00%> (-1.00%)
...ver/storage/redis/session/RedisSessionManager.java 29.31% <0.00%> (-0.52%) 10.00% <0.00%> (ø%)
... and 29 more

map.putIfAbsent(serverAddress, new LinkedBlockingQueue<>());
basket = map.get(serverAddress);
basketMap.computeIfAbsent(serverAddress, k -> new LinkedBlockingQueue<>());
if (!basketMap.get(serverAddress).offer(rpcMessage)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not need to get again

BlockingQueue<RpcMessage> basket = basketMap.computeIfAbsent(serverAddress, k -> new LinkedBlockingQueue<>());
if (!basket .offer(rpcMessage)) {
    ......
}

liubin01 added 2 commits October 14, 2020 11:02
* origin/develop:
  optimize: Local variable 'map' is redundant and check queue offer return value apache#3187
@funky-eyes
Copy link
Contributor

请绑定一下githubid到你本地的git插件上

@everyhook1 everyhook1 closed this Oct 15, 2020
@everyhook1 everyhook1 reopened this Oct 15, 2020
Copy link
Contributor

@funky-eyes funky-eyes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@slievrly slievrly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@funky-eyes
Copy link
Contributor

funky-eyes commented Oct 20, 2020

解决一下冲突,再绑定一下githubid到你本地的git插件上push一下

@wangliang181230 wangliang181230 added this to the 1.5.0 milestone Oct 29, 2020
@funky-eyes funky-eyes changed the title optimize: Local variable 'map' is redundant and check queue offer return value #3187 optimize: Local variable 'map' is redundant and check queue offer return value Nov 11, 2020
@funky-eyes funky-eyes merged commit 69f341a into apache:develop Nov 11, 2020
LiWenGu pushed a commit to LiWenGu/seata that referenced this pull request Dec 4, 2020
@wangliang181230 wangliang181230 modified the milestones: 1.5.0, 1.4.1 Dec 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module/core core module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

optimize: Local variable 'map' is redundant and check queue offer return value
6 participants