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.

Sign up for GitHub

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

Jump to bottom

[Enhancement] NettyRemotingClient#createChannelAsync一开始的get操作是冗余的 #9081

Open
1 task done
PleaseGiveMeTheCoke opened this issue Dec 26, 2024 · 5 comments · May be fixed by #9104
Open
1 task done

[Enhancement] NettyRemotingClient#createChannelAsync一开始的get操作是冗余的 #9081

PleaseGiveMeTheCoke opened this issue Dec 26, 2024 · 5 comments · May be fixed by #9104
Labels
type/enhancement

Comments

@PleaseGiveMeTheCoke
Copy link

Before Creating the Enhancement Request

  • I have confirmed that this should be classified as an enhancement rather than a bug/feature.

Summary

createChannelAsync方法的一开始 ，执行了get操作
image
而createChannelAsync方法的调用方getAndCreateChannelAsync方法中，已经执行过同样的get操作了。这段 get 操作不应该在 createChannelAsync方法中进行，因为这个方法的命名是 createChannelAsync。

image

Motivation

  1. 目前getAndCreateChannelAsync方法相当于执行了两次同样的get操作
  2. 目前createChannelAsync的方法命名和具体职责存在不一致
  3. 优化后可以提升性能（少了一次get操作），提高代码的可读性

Describe the Solution You'd Like

删除下面的代码
image

并在该方法的另一处调用方getAndCreateNameserverChannelAsync中，调用之前加上get操作

image

Describe Alternatives You've Considered

无

Additional Context

No response
@qianye1001
Copy link
Contributor

Double-Check Locking is the standard process for preventing concurrent creation. Creation only occurs once when the connection is established. Reducing one in-memory check won't provide any significant performance improvement.

@qianye1001
Copy link
Contributor

It is more recommended to remove check in getAndCreateChannelAsync

@Willhow-Gao
Copy link
Contributor

create前先进行前置检查是有必要的，这个逻辑放在create方法里我理解没啥问题，对调用方更友好

@PleaseGiveMeTheCoke
Copy link
Author

@qianye1001 Is it necessary to submit a PR to remove the check in getAndCreateChannelAsync?

@qianye1001
Copy link
Contributor

@qianye1001 Is it necessary to submit a PR to remove the check in getAndCreateChannelAsync?

You can try

@PleaseGiveMeTheCoke PleaseGiveMeTheCoke linked a pull request Jan 4, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type/enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[ISSUE #9081] Remove redundant get PleaseGiveMeTheCoke/rocketmq
3 participants
@qianye1001 @Willhow-Gao @PleaseGiveMeTheCoke