Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 95 additions & 23 deletions common/url.go
Original file line number Diff line number Diff line change
Expand Up @@ -820,7 +820,32 @@

// ToMap transfer URL to Map
func (c *URL) ToMap() map[string]string {
paramsMap := make(map[string]string)
c.paramsLock.RLock()
paramCount := len(c.params)
c.paramsLock.RUnlock()

fixedCount := 0
if c.Protocol != "" {
fixedCount++
}
if c.Username != "" {
fixedCount++
}
if c.Password != "" {
fixedCount++
}
if c.Location != "" {
fixedCount += 2 // host + port
}
if c.Path != "" {
fixedCount++
}

if paramCount == 0 && fixedCount == 0 {
return nil
}
Comment on lines +823 to +846

paramsMap := make(map[string]string, paramCount+fixedCount)
Comment on lines -912 to +848

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  • paramCount 单独加锁读一次,后面 RangeParams 又加锁遍历一次,多了一次锁操作。
  • 这不是一致性快照:两次锁之间 params 可能变,容量估算可能不准。
  • fixedCount 只是容量估算,不影响逻辑;估多了也只是多分一点内存。
  • 如果 params 里本来就有 protocolhostport 这些 key,后面的固定字段会覆盖它们,fixedCount 仍然按新增 key 算,容量也会估大。
  • 性能收益主要来自预分配,但可读性代价有点高。

建议就是保留容量估算,去掉无意义初始化,并确保 protocol/location 行为不变。


c.RangeParams(
func(key, value string) bool {
Expand All @@ -839,24 +864,18 @@
paramsMap["password"] = c.Password
}
if c.Location != "" {
paramsMap["host"] = strings.Split(c.Location, ":")[0]
var port string
if strings.Contains(c.Location, ":") {
port = strings.Split(c.Location, ":")[1]
// Single-pass split avoids repeated string scanning.
parts := strings.Split(c.Location, ":")
paramsMap["host"] = parts[0]
if len(parts) > 1 {
paramsMap["port"] = parts[len(parts)-1]
Comment on lines +867 to +871
} else {
port = "0"
paramsMap["port"] = "0"
}
Comment on lines 866 to 874

@Alanxtl Alanxtl Jun 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ToMap 的多地址 port 解析语义被改了

原逻辑对 127.0.0.1:2181,127.0.0.1:2182,127.0.0.1:2183port2181,127.0.0.1 确实不合理,
PR 改成取最后一个 : 后的 2183。也不合理

这里改成合理的逻辑,然后在pr简介里面说明

paramsMap["port"] = port
}
if c.Protocol != "" {
paramsMap[PROTOCOL] = c.Protocol
}
if c.Path != "" {
paramsMap["path"] = c.Path
}
if len(paramsMap) == 0 {
return nil
}
return paramsMap
}

Expand Down Expand Up @@ -1011,7 +1030,7 @@
}

// IsEquals compares if two URLs equals with each other. Excludes are all parameter keys which should ignored.
func IsEquals(left *URL, right *URL, excludes ...string) bool {

Check failure on line 1033 in common/url.go

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 35 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=apache_dubbo-go&issues=AZ651ejwrnf4cV9Q3M_u&open=AZ651ejwrnf4cV9Q3M_u&pullRequest=3402
if (left == nil && right != nil) || (right == nil && left != nil) {
return false
}
Expand All @@ -1019,23 +1038,76 @@
return false
}

leftMap := left.ToMap()
rightMap := right.ToMap()
for _, exclude := range excludes {
delete(leftMap, exclude)
delete(rightMap, exclude)
// Build a small lookup set for excluded keys to avoid repeated linear scans
// and to avoid materializing two full maps just for comparison.
var excludeSet map[string]struct{}
if len(excludes) > 0 {
excludeSet = make(map[string]struct{}, len(excludes))
for _, k := range excludes {
excludeSet[k] = struct{}{}
}
}

if len(leftMap) != len(rightMap) {
return false
isExcluded := func(key string) bool {
if excludeSet == nil {
return false
}
_, ok := excludeSet[key]
return ok
}

for lk, lv := range leftMap {
if rv, ok := rightMap[lk]; !ok {
// Compare scalar fields directly.
if left.Protocol != right.Protocol && !isExcluded(PROTOCOL) {
return false
}
if left.Username != right.Username && !isExcluded("username") {
return false
}
if left.Password != right.Password && !isExcluded("password") {
return false
}
if left.Path != right.Path && !isExcluded("path") {
return false
}
if left.Location != right.Location {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P1] 这里把 host / port 的排除逻辑挂在整条 Location 上了:只传 host 或只传 port 时,left.Location != right.Location 也会直接落入“放行”分支,另一半地址差异被一并忽略。旧实现是按 ToMap() 删除单个 key 比较,两个字段的排除语义是独立的。请分别比较 host/port,或保留原来的按键删除语义。

if isExcluded("host") && isExcluded("port") {
// Both excluded, skip Location comparison
} else if isExcluded("host") || isExcluded("port") {
// Only one side of host:port excluded — fall through to param comparison
Comment on lines +1072 to +1076
} else {
return false
} else if lv != rv {
}
}
Comment on lines +1072 to +1080

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

IsEquals 在只 exclude hostport 时会漏比较另一半 location

PR 里当 left.Location != right.Location 且只排除 hostport 其中一个时,代码直接“fall through”,但后面只比较 params,不再比较未被排除的 host/port。

你试一下Location: 10.0.0.1:20880 vs 10.0.0.2:20880,exclude port 后 PR 返回 true,base 返回 false


// Compare params directly without materializing maps.
// Build left params (excluding filtered keys), then verify right matches.
leftParams := make(map[string]string)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

这里可以做个容量预估,len(left.params) 就有了

left.RangeParams(func(key, value string) bool {
if !isExcluded(key) {
leftParams[key] = value
}
return true
})

rightCount := 0
mismatch := false
right.RangeParams(func(key, value string) bool {
if isExcluded(key) {
return true
}
rightCount++
if lv, ok := leftParams[key]; !ok || lv != value {
mismatch = true
return false
}
return true
})

if mismatch {
return false
}
if rightCount != len(leftParams) {
return false
}

return true
Expand Down
79 changes: 79 additions & 0 deletions common/url_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1507,3 +1507,82 @@ func TestCloneThreadSafe(t *testing.T) {

wg.Wait()
}

// --- Benchmarks for IsEquals and ToMap ---

func BenchmarkIsEquals_SmallParams(b *testing.B) {
u1, _ := NewURL("dubbo://127.0.0.1:20000/com.test.Service?key1=value1&key2=value2")
u2, _ := NewURL("dubbo://127.0.0.1:20000/com.test.Service?key1=value1&key2=value2")
b.ResetTimer()
for i := 0; i < b.N; i++ {
IsEquals(u1, u2)
}
}

func BenchmarkIsEquals_LargeParams(b *testing.B) {
base1 := "dubbo://127.0.0.1:20000/com.test.Service?"
base2 := "dubbo://127.0.0.1:20000/com.test.Service?"
for i := 0; i < 50; i++ {
kv := "key" + strconv.Itoa(i) + "=val" + strconv.Itoa(i)
if i > 0 {
base1 += "&"
base2 += "&"
}
base1 += kv
base2 += kv
}
u1, _ := NewURL(base1)
u2, _ := NewURL(base2)
b.ResetTimer()
for i := 0; i < b.N; i++ {
IsEquals(u1, u2)
}
}

func BenchmarkIsEquals_WithExcludes(b *testing.B) {
u1, _ := NewURL("dubbo://127.0.0.1:20000/com.test.Service?key1=value1&key2=different&key3=value3")
u2, _ := NewURL("dubbo://127.0.0.1:20000/com.test.Service?key1=value1&key2=other&key3=value3")
b.ResetTimer()
for i := 0; i < b.N; i++ {
IsEquals(u1, u2, "key2")
}
}

func BenchmarkToMap_SmallParams(b *testing.B) {
u, _ := NewURL("dubbo://127.0.0.1:20000/com.test.Service?key1=value1&key2=value2")
u.Username = "user"
u.Password = "pass"
b.ResetTimer()
for i := 0; i < b.N; i++ {
_ = u.ToMap()
}
}

func BenchmarkToMap_LargeParams(b *testing.B) {
base := "dubbo://127.0.0.1:20000/com.test.Service?"
for i := 0; i < 50; i++ {
if i > 0 {
base += "&"
}
base += "key" + strconv.Itoa(i) + "=val" + strconv.Itoa(i)
}
u, _ := NewURL(base)
u.Username = "user"
u.Password = "pass"
b.ResetTimer()
for i := 0; i < b.N; i++ {
_ = u.ToMap()
}
}

func BenchmarkToMap_EmptyParams(b *testing.B) {
u := &URL{
Protocol: "dubbo",
Location: "127.0.0.1:20000",
Path: "/com.test.Service",
}
b.ResetTimer()
for i := 0; i < b.N; i++ {
_ = u.ToMap()
}
}
Comment on lines +1511 to +1588

@Alanxtl Alanxtl Jun 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

不用自己写benchmark 直接复用已有的benchmark 或者在benchmark文件里面添加你需要的benchmark

Loading