You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "ywxzm03 (via GitHub)" <gi...@apache.org> on 2026/06/12 12:14:16 UTC
[PR] perf(common): optimize URL.String() locking and string building [dubbo-go]
ywxzm03 opened a new pull request, #3404:
URL: https://github.com/apache/dubbo-go/pull/3404
**What this PR does**:
Optimizes lock usage and string construction in `common.URL.String()`, making it lighter-weight and less blocking while keeping the existing output exactly unchanged.
This change includes:
- **Locking**: changes `paramsLock.Lock()` to `paramsLock.RLock()`, because this method only reads params. A read lock allows multiple concurrent `String()` calls while still remaining mutually exclusive with writes.
- **String construction**: replaces `fmt.Sprintf` with `strings.Builder` + `Grow` + `WriteString`. By precomputing the required capacity and allocating once, this removes intermediate strings and repeated growth.
**Which issue(s) this PR fixes**:
Fixes #3393
**Special notes for your reviewer**:
- The benchmark reuses the existing `BenchmarkURLString` on develop (`common/url_benchmark_test.go`, from #3399). Comparison results from `go test ./common/ -bench BenchmarkURLString -benchmem` on Apple M5:
| Case | Before | After |
| ----------- | ------------------------------------- | ----------------------------------------- |
| params_1 | 383.2 ns/op, 728 B/op, 13 allocs/op | 309.7 ns/op, 536 B/op, **7 allocs/op** |
| params_32 | 1651 ns/op, 3385 B/op, 16 allocs/op | 1804 ns/op, 3192 B/op, **10 allocs/op** |
| params_256 | 18138 ns/op, 21698 B/op, 20 allocs/op | 18499 ns/op, 21496 B/op, **14 allocs/op** |
| params_1024 | 95501 ns/op, 98025 B/op, 25 allocs/op | 91318 ns/op, 97784 B/op, **19 allocs/op** |
Latency drops noticeably for small URLs. For large params cases, runtime is mostly dominated by `params.Encode()`, so latency is roughly unchanged, but the allocation count still decreases.
- `go test ./common/` passes.
**Does this PR introduce a user-facing change?**
```release-note
NONE
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] perf(common): optimize URL.String() locking and string building [dubbo-go]
Posted by "ywxzm03 (via GitHub)" <gi...@apache.org>.
ywxzm03 commented on code in PR #3404:
URL: https://github.com/apache/dubbo-go/pull/3404#discussion_r3407538735
##########
common/url.go:
##########
@@ -395,21 +395,51 @@ func isMatchCategory(category1 string, category2 string) bool {
}
func (c *URL) String() string {
- c.paramsLock.Lock()
- defer c.paramsLock.Unlock()
+ c.paramsLock.RLock()
+ defer c.paramsLock.RUnlock()
+
+ encodedParams := c.params.Encode()
var buf strings.Builder
- if len(c.Username) == 0 && len(c.Password) == 0 {
- buf.WriteString(fmt.Sprintf("%s://%s:%s%s", c.Protocol, c.Ip, c.Port, c.Path))
- } else {
- buf.WriteString(fmt.Sprintf("%s://%s:%s@%s:%s%s", c.Protocol, c.Username, c.Password, c.Ip, c.Port, c.Path))
+ size := c.stringPrefixLen()
+ if encodedParams != "" {
+ size += len("?") + len(encodedParams)
}
- if encoded := c.params.Encode(); encoded != "" {
+ buf.Grow(size)
+ c.writeStringPrefix(&buf)
+ if encodedParams != "" {
buf.WriteByte('?')
- buf.WriteString(encoded)
+ buf.WriteString(encodedParams)
}
return buf.String()
}
+func (c *URL) stringPrefixLen() int {
+ size := len(c.Protocol) + len("://") + len(c.Ip) + len(":") + len(c.Port) + len(c.Path)
+ if c.hasAuth() {
+ size += len(c.Username) + len(":") + len(c.Password) + len("@")
+ }
+ return size
+}
Review Comment:
fixed
##########
common/url.go:
##########
@@ -395,21 +395,51 @@ func isMatchCategory(category1 string, category2 string) bool {
}
func (c *URL) String() string {
- c.paramsLock.Lock()
- defer c.paramsLock.Unlock()
+ c.paramsLock.RLock()
+ defer c.paramsLock.RUnlock()
+
+ encodedParams := c.params.Encode()
var buf strings.Builder
- if len(c.Username) == 0 && len(c.Password) == 0 {
- buf.WriteString(fmt.Sprintf("%s://%s:%s%s", c.Protocol, c.Ip, c.Port, c.Path))
- } else {
- buf.WriteString(fmt.Sprintf("%s://%s:%s@%s:%s%s", c.Protocol, c.Username, c.Password, c.Ip, c.Port, c.Path))
+ size := c.stringPrefixLen()
+ if encodedParams != "" {
+ size += len("?") + len(encodedParams)
}
- if encoded := c.params.Encode(); encoded != "" {
+ buf.Grow(size)
+ c.writeStringPrefix(&buf)
+ if encodedParams != "" {
buf.WriteByte('?')
- buf.WriteString(encoded)
+ buf.WriteString(encodedParams)
}
return buf.String()
}
+func (c *URL) stringPrefixLen() int {
+ size := len(c.Protocol) + len("://") + len(c.Ip) + len(":") + len(c.Port) + len(c.Path)
+ if c.hasAuth() {
+ size += len(c.Username) + len(":") + len(c.Password) + len("@")
+ }
+ return size
+}
+
+func (c *URL) writeStringPrefix(buf *strings.Builder) {
+ buf.WriteString(c.Protocol)
+ buf.WriteString("://")
+ if c.hasAuth() {
+ buf.WriteString(c.Username)
+ buf.WriteString(":")
+ buf.WriteString(c.Password)
+ buf.WriteString("@")
+ }
+ buf.WriteString(c.Ip)
+ buf.WriteString(":")
+ buf.WriteString(c.Port)
+ buf.WriteString(c.Path)
+}
Review Comment:
fixed
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] perf(common): optimize URL.String() locking and string building [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3404:
URL: https://github.com/apache/dubbo-go/pull/3404#issuecomment-4691172010
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3404) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3404&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3404&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3404&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3404&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3404&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3404)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] perf(common): optimize URL.String() locking and string building [dubbo-go]
Posted by "Alanxtl (via GitHub)" <gi...@apache.org>.
Alanxtl merged PR #3404:
URL: https://github.com/apache/dubbo-go/pull/3404
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] perf(common): optimize URL.String() locking and string building [dubbo-go]
Posted by "ywxzm03 (via GitHub)" <gi...@apache.org>.
ywxzm03 commented on code in PR #3404:
URL: https://github.com/apache/dubbo-go/pull/3404#discussion_r3407539048
##########
common/url.go:
##########
@@ -395,21 +395,51 @@ func isMatchCategory(category1 string, category2 string) bool {
}
func (c *URL) String() string {
- c.paramsLock.Lock()
- defer c.paramsLock.Unlock()
+ c.paramsLock.RLock()
+ defer c.paramsLock.RUnlock()
+
+ encodedParams := c.params.Encode()
var buf strings.Builder
- if len(c.Username) == 0 && len(c.Password) == 0 {
- buf.WriteString(fmt.Sprintf("%s://%s:%s%s", c.Protocol, c.Ip, c.Port, c.Path))
- } else {
- buf.WriteString(fmt.Sprintf("%s://%s:%s@%s:%s%s", c.Protocol, c.Username, c.Password, c.Ip, c.Port, c.Path))
+ size := c.stringPrefixLen()
+ if encodedParams != "" {
+ size += len("?") + len(encodedParams)
}
- if encoded := c.params.Encode(); encoded != "" {
+ buf.Grow(size)
+ c.writeStringPrefix(&buf)
+ if encodedParams != "" {
buf.WriteByte('?')
- buf.WriteString(encoded)
+ buf.WriteString(encodedParams)
}
return buf.String()
}
+func (c *URL) stringPrefixLen() int {
+ size := len(c.Protocol) + len("://") + len(c.Ip) + len(":") + len(c.Port) + len(c.Path)
+ if c.hasAuth() {
+ size += len(c.Username) + len(":") + len(c.Password) + len("@")
+ }
+ return size
+}
+
+func (c *URL) writeStringPrefix(buf *strings.Builder) {
+ buf.WriteString(c.Protocol)
+ buf.WriteString("://")
+ if c.hasAuth() {
+ buf.WriteString(c.Username)
+ buf.WriteString(":")
+ buf.WriteString(c.Password)
+ buf.WriteString("@")
+ }
+ buf.WriteString(c.Ip)
+ buf.WriteString(":")
+ buf.WriteString(c.Port)
+ buf.WriteString(c.Path)
+}
+
+func (c *URL) hasAuth() bool {
+ return len(c.Username) != 0 || len(c.Password) != 0
+}
Review Comment:
fixed
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] perf(common): optimize URL.String() locking and string building [dubbo-go]
Posted by "Alanxtl (via GitHub)" <gi...@apache.org>.
Alanxtl commented on PR #3404:
URL: https://github.com/apache/dubbo-go/pull/3404#issuecomment-4697577205
@Vanillaxi 可以互相review一下 :)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] perf(common): optimize URL.String() locking and string building [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3404:
URL: https://github.com/apache/dubbo-go/pull/3404#issuecomment-4697732293
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3404) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3404&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3404&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3404&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3404&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3404&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3404)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] perf(common): optimize URL.String() locking and string building [dubbo-go]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #3404:
URL: https://github.com/apache/dubbo-go/pull/3404#issuecomment-4691211930
## [Codecov](https://app.codecov.io/gh/apache/dubbo-go/pull/3404?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 53.15%. Comparing base ([`60d1c2a`](https://app.codecov.io/gh/apache/dubbo-go/commit/60d1c2a949f0ee0da4be3fe09fb79d295491e040?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`4b03cd1`](https://app.codecov.io/gh/apache/dubbo-go/commit/4b03cd1713825d857fd4fbb312320ff3bc5731dd?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 832 commits behind head on develop.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## develop #3404 +/- ##
===========================================
+ Coverage 46.76% 53.15% +6.38%
===========================================
Files 295 493 +198
Lines 17172 38210 +21038
===========================================
+ Hits 8031 20309 +12278
- Misses 8287 16267 +7980
- Partials 854 1634 +780
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/dubbo-go/pull/3404?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] perf(common): optimize URL.String() locking and string building [dubbo-go]
Posted by "Alanxtl (via GitHub)" <gi...@apache.org>.
Alanxtl commented on code in PR #3404:
URL: https://github.com/apache/dubbo-go/pull/3404#discussion_r3407486568
##########
common/url.go:
##########
@@ -395,21 +395,51 @@ func isMatchCategory(category1 string, category2 string) bool {
}
func (c *URL) String() string {
- c.paramsLock.Lock()
- defer c.paramsLock.Unlock()
+ c.paramsLock.RLock()
+ defer c.paramsLock.RUnlock()
+
+ encodedParams := c.params.Encode()
var buf strings.Builder
- if len(c.Username) == 0 && len(c.Password) == 0 {
- buf.WriteString(fmt.Sprintf("%s://%s:%s%s", c.Protocol, c.Ip, c.Port, c.Path))
- } else {
- buf.WriteString(fmt.Sprintf("%s://%s:%s@%s:%s%s", c.Protocol, c.Username, c.Password, c.Ip, c.Port, c.Path))
+ size := c.stringPrefixLen()
+ if encodedParams != "" {
+ size += len("?") + len(encodedParams)
}
- if encoded := c.params.Encode(); encoded != "" {
+ buf.Grow(size)
+ c.writeStringPrefix(&buf)
+ if encodedParams != "" {
buf.WriteByte('?')
- buf.WriteString(encoded)
+ buf.WriteString(encodedParams)
}
return buf.String()
}
+func (c *URL) stringPrefixLen() int {
+ size := len(c.Protocol) + len("://") + len(c.Ip) + len(":") + len(c.Port) + len(c.Path)
+ if c.hasAuth() {
+ size += len(c.Username) + len(":") + len(c.Password) + len("@")
+ }
+ return size
+}
+
+func (c *URL) writeStringPrefix(buf *strings.Builder) {
+ buf.WriteString(c.Protocol)
+ buf.WriteString("://")
+ if c.hasAuth() {
+ buf.WriteString(c.Username)
+ buf.WriteString(":")
+ buf.WriteString(c.Password)
+ buf.WriteString("@")
+ }
+ buf.WriteString(c.Ip)
+ buf.WriteString(":")
+ buf.WriteString(c.Port)
+ buf.WriteString(c.Path)
+}
+
+func (c *URL) hasAuth() bool {
+ return len(c.Username) != 0 || len(c.Password) != 0
+}
Review Comment:
这个就不用单独抽象一个函数了
##########
common/url.go:
##########
@@ -395,21 +395,51 @@ func isMatchCategory(category1 string, category2 string) bool {
}
func (c *URL) String() string {
- c.paramsLock.Lock()
- defer c.paramsLock.Unlock()
+ c.paramsLock.RLock()
+ defer c.paramsLock.RUnlock()
+
+ encodedParams := c.params.Encode()
var buf strings.Builder
- if len(c.Username) == 0 && len(c.Password) == 0 {
- buf.WriteString(fmt.Sprintf("%s://%s:%s%s", c.Protocol, c.Ip, c.Port, c.Path))
- } else {
- buf.WriteString(fmt.Sprintf("%s://%s:%s@%s:%s%s", c.Protocol, c.Username, c.Password, c.Ip, c.Port, c.Path))
+ size := c.stringPrefixLen()
+ if encodedParams != "" {
+ size += len("?") + len(encodedParams)
}
- if encoded := c.params.Encode(); encoded != "" {
+ buf.Grow(size)
+ c.writeStringPrefix(&buf)
+ if encodedParams != "" {
buf.WriteByte('?')
- buf.WriteString(encoded)
+ buf.WriteString(encodedParams)
}
return buf.String()
}
+func (c *URL) stringPrefixLen() int {
+ size := len(c.Protocol) + len("://") + len(c.Ip) + len(":") + len(c.Port) + len(c.Path)
+ if c.hasAuth() {
+ size += len(c.Username) + len(":") + len(c.Password) + len("@")
+ }
+ return size
+}
+
+func (c *URL) writeStringPrefix(buf *strings.Builder) {
+ buf.WriteString(c.Protocol)
+ buf.WriteString("://")
+ if c.hasAuth() {
+ buf.WriteString(c.Username)
+ buf.WriteString(":")
+ buf.WriteString(c.Password)
+ buf.WriteString("@")
+ }
+ buf.WriteString(c.Ip)
+ buf.WriteString(":")
+ buf.WriteString(c.Port)
+ buf.WriteString(c.Path)
+}
Review Comment:
这个也不用单独抽象函数了 又没有别的地方用
##########
common/url.go:
##########
@@ -395,21 +395,51 @@ func isMatchCategory(category1 string, category2 string) bool {
}
func (c *URL) String() string {
- c.paramsLock.Lock()
- defer c.paramsLock.Unlock()
+ c.paramsLock.RLock()
+ defer c.paramsLock.RUnlock()
+
+ encodedParams := c.params.Encode()
var buf strings.Builder
- if len(c.Username) == 0 && len(c.Password) == 0 {
- buf.WriteString(fmt.Sprintf("%s://%s:%s%s", c.Protocol, c.Ip, c.Port, c.Path))
- } else {
- buf.WriteString(fmt.Sprintf("%s://%s:%s@%s:%s%s", c.Protocol, c.Username, c.Password, c.Ip, c.Port, c.Path))
+ size := c.stringPrefixLen()
+ if encodedParams != "" {
+ size += len("?") + len(encodedParams)
}
- if encoded := c.params.Encode(); encoded != "" {
+ buf.Grow(size)
+ c.writeStringPrefix(&buf)
+ if encodedParams != "" {
buf.WriteByte('?')
- buf.WriteString(encoded)
+ buf.WriteString(encodedParams)
}
return buf.String()
}
+func (c *URL) stringPrefixLen() int {
+ size := len(c.Protocol) + len("://") + len(c.Ip) + len(":") + len(c.Port) + len(c.Path)
+ if c.hasAuth() {
+ size += len(c.Username) + len(":") + len(c.Password) + len("@")
+ }
+ return size
+}
Review Comment:
这个也不用
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org