-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
fix: use path.Join
replace fmt.Sprintf
to set the path value
#3163
Conversation
Signed-off-by: WillardHu <wei.hu@daocloud.io>
@@ -60,20 +61,20 @@ func (factory *eventbusFactory) GetSource(ep *v1.RuleEndpoint, sourceResource ma | |||
} | |||
|
|||
func (eb *EventBus) RegisterListener(handle listener.Handle) error { | |||
listener.MessageHandlerInstance.AddListener(fmt.Sprintf("%s/node/%s/%s/%s", "bus", eb.nodeName, eb.namespace, eb.subTopic), handle) | |||
listener.MessageHandlerInstance.AddListener(path.Join("bus/node", eb.nodeName, eb.namespace, eb.subTopic), handle) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you pls share some data on how the performance got improved?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
var s1t time.Time
var used time.Duration
s1t = time.Now()
for i := 0; i < 100000; i++ {
fmt.Sprintf("node/%s/%s/%s", "nodeName", "namespace", "subTopic")
}
used = time.Since(s1t)
fmt.Println("time cost of `sprintf`:", used)
s1t = time.Now()
for i := 0; i < 100000; i++ {
path.Join("node", "nodeName", "namespace", "subTopic")
}
used = time.Since(s1t)
fmt.Println("time cost of `join`:", used)
Run 3 times pirnt out:
=== RUN TestPathJoin
time cost of `sprintf`: 15.840042ms
time cost of `join`: 12.771455ms
--- PASS: TestPathJoin (0.03s)
=== RUN TestPathJoin
time cost of `sprintf`: 14.852592ms
time cost of `join`: 12.130716ms
--- PASS: TestPathJoin (0.03s)
=== RUN TestPathJoin
time cost of `sprintf`: 19.307545ms
time cost of `join`: 14.843423ms
--- PASS: TestPathJoin (0.03s)
/lgtm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fisherxu The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Signed-off-by: WillardHu wei.hu@daocloud.io
What type of PR is this?
/kind bug
What this PR does / why we need it:
Better performance and readability
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: