This is an automated email from the ASF dual-hosted git repository.
Alanxtl pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/dubbo-go.git
The following commit(s) were added to refs/heads/develop by this push:
new f7f350cfe perf(common): reduce URL CloneWithFilter and MergeURL params
copying (#3403)
f7f350cfe is described below
commit f7f350cfe4fbcfe35d609306a48b4f72de5b923d
Author: ヴァニラシ <[email protected]>
AuthorDate: Sat Jun 13 13:49:54 2026 +0800
perf(common): reduce URL CloneWithFilter and MergeURL params copying (#3403)
---
common/url.go | 196 +++++++++++++++++++++++++++++++++++------------------
common/url_test.go | 64 +++++++++++++++++
2 files changed, 195 insertions(+), 65 deletions(-)
diff --git a/common/url.go b/common/url.go
index d6a800e22..462f4ee36 100644
--- a/common/url.go
+++ b/common/url.go
@@ -678,23 +678,34 @@ func (c *URL) GetParams() url.Values {
return c.CopyParams()
}
-// CopyParams returns a deep copy of params.
-func (c *URL) CopyParams() url.Values {
- c.paramsLock.RLock()
- defer c.paramsLock.RUnlock()
-
- if c.params == nil {
+func copyURLValues(src url.Values) url.Values {
+ if src == nil {
return nil
}
- params := make(url.Values, len(c.params))
- for k, vs := range c.params {
+ dst := make(url.Values, len(src))
+ for k, vs := range src {
copied := make([]string, len(vs))
copy(copied, vs)
- params[k] = copied
+ dst[k] = copied
}
- return params
+ return dst
+}
+
+func valuesHasNonDefaultParam(values url.Values, key string) bool {
+ if len(values) == 0 {
+ return false
+ }
+ return values.Get(key) != ""
+}
+
+// CopyParams returns a deep copy of params.
+func (c *URL) CopyParams() url.Values {
+ c.paramsLock.RLock()
+ defer c.paramsLock.RUnlock()
+
+ return copyURLValues(c.params)
}
// GetParamAndDecoded gets values and decode
@@ -880,41 +891,53 @@ func (c *URL) ToMap() map[string]string {
func (c *URL) MergeURL(anotherUrl *URL) *URL {
// After Clone, it is a new URL that there is no thread safe issue.
mergedURL := c.Clone()
- params := mergedURL.GetParams()
- // iterator the anotherUrl if c not have the key ,merge in
- // anotherUrl usually will not changed. so change RangeParams to
GetParams to avoid the string value copy.// Group get group
- for key, value := range anotherUrl.GetParams() {
- if _, ok := mergedURL.GetNonDefaultParam(key); !ok {
- if len(value) > 0 {
- params[key] = make([]string, len(value))
- copy(params[key], value)
+ params := mergedURL.params
+ if params == nil {
+ params = url.Values{}
+ mergedURL.params = params
+ }
+ baseTimestamp := params.Get(constant.TimestampKey)
+ baseHasTimestamp := baseTimestamp != ""
+
+ func() {
+ anotherUrl.paramsLock.RLock()
+ defer anotherUrl.paramsLock.RUnlock()
+
+ // Merge params from anotherUrl under one read lock.
+ for key, value := range anotherUrl.params {
+ if !valuesHasNonDefaultParam(params, key) {
+ if len(value) > 0 {
+ params[key] = make([]string, len(value))
+ copy(params[key], value)
+ }
}
}
- }
-
- // remote timestamp
- if v, ok := c.GetNonDefaultParam(constant.TimestampKey); !ok {
- params[constant.RemoteTimestampKey] = []string{v}
- params[constant.TimestampKey] =
[]string{anotherUrl.GetParam(constant.TimestampKey, "")}
- }
- // finally execute methodConfigMergeFcn
- mergedURL.Methods = make([]string, len(anotherUrl.Methods))
- for i, method := range anotherUrl.Methods {
- for _, paramKey := range []string{constant.LoadbalanceKey,
constant.ClusterKey, constant.RetriesKey, constant.TimeoutKey} {
- if v := anotherUrl.GetParam(paramKey, ""); len(v) > 0 {
- params[paramKey] = []string{v}
- }
+ // remote timestamp
+ if !baseHasTimestamp {
+ params[constant.RemoteTimestampKey] =
[]string{baseTimestamp}
+ params[constant.TimestampKey] =
[]string{anotherUrl.params.Get(constant.TimestampKey)}
+ }
- methodsKey := "methods." + method + "." + paramKey
- // if len(mergedURL.GetParam(methodsKey, "")) == 0 {
- if v := anotherUrl.GetParam(methodsKey, ""); len(v) > 0
{
- params[methodsKey] = []string{v}
+ // finally execute methodConfigMergeFcn
+ mergedURL.Methods = make([]string, len(anotherUrl.Methods))
+ for i, method := range anotherUrl.Methods {
+ for _, paramKey := range
[]string{constant.LoadbalanceKey, constant.ClusterKey, constant.RetriesKey,
constant.TimeoutKey} {
+ if v := anotherUrl.params.Get(paramKey); len(v)
> 0 {
+ params[paramKey] = []string{v}
+ }
+
+ methodsKey := "methods." + method + "." +
paramKey
+ // if len(mergedURL.GetParam(methodsKey, ""))
== 0 {
+ if v := anotherUrl.params.Get(methodsKey);
len(v) > 0 {
+ params[methodsKey] = []string{v}
+ }
+ // }
+ mergedURL.Methods[i] = method
}
- // }
- mergedURL.Methods[i] = method
}
- }
+ }()
+
// merge attributes
anotherUrl.RangeAttributes(func(attrK string, attrV any) bool {
if _, ok := mergedURL.GetAttribute(attrK); !ok {
@@ -931,7 +954,27 @@ func (c *URL) MergeURL(anotherUrl *URL) *URL {
// excludeParams: the set of parameters to exclude from the cloned URL
// reserveParams: the set of parameters to retain in the cloned URL
func (c *URL) CloneWithFilter(excludeParams *gxset.HashSet, reserveParams
[]string) *URL {
- newURL := &URL{
+ newURL := c.newURLForClone()
+ newURL.params = c.copyFilteredParams(newURL.params, excludeParams,
reserveParams)
+
+ // Copy attributes
+ c.RangeAttributes(
+ func(key string, value any) bool {
+ newURL.SetAttribute(key, value)
+ return true
+ },
+ )
+
+ // Copy SubURL if it exists
+ if c.SubURL != nil {
+ newURL.SubURL = c.SubURL.Clone()
+ }
+
+ return newURL
+}
+
+func (c *URL) newURLForClone() *URL {
+ return &URL{
Protocol: c.Protocol,
Location: c.Location,
Ip: c.Ip,
@@ -945,37 +988,60 @@ func (c *URL) CloneWithFilter(excludeParams
*gxset.HashSet, reserveParams []stri
attributes: make(map[string]any),
params: url.Values{},
}
+}
- // Copy and filter params based on excludeParams or reserveParams
- c.RangeParams(
- func(key, value string) bool {
- // If the param is in excludeParams or not in
reserveParams, skip it
- if excludeParams != nil && excludeParams.Contains(key) {
- return true
- }
- if len(reserveParams) > 0 &&
!slices.Contains(reserveParams, key) {
- return true
+func (c *URL) copyFilteredParams(params url.Values, excludeParams
*gxset.HashSet, reserveParams []string) url.Values {
+ c.paramsLock.RLock()
+ defer c.paramsLock.RUnlock()
+
+ if excludeParams == nil && len(reserveParams) == 0 {
+ if len(c.params) > 8 {
+ copiedParams := copyURLValues(c.params)
+ if copiedParams != nil {
+ return copiedParams
}
- // Set the param if it passes the filter
- newURL.SetParam(key, value)
- return true
- },
- )
+ }
+ for key, values := range c.params {
+ copied := make([]string, len(values))
+ copy(copied, values)
+ params[key] = copied
+ }
+ return params
+ }
- // Copy attributes
- c.RangeAttributes(
- func(key string, value any) bool {
- newURL.SetAttribute(key, value)
- return true
- },
- )
+ if capacity := paramsCapacityForClone(len(c.params), excludeParams,
reserveParams); capacity > 8 {
+ params = make(url.Values, capacity)
+ }
- // Copy SubURL if it exists
- if c.SubURL != nil {
- newURL.SubURL = c.SubURL.Clone()
+ for key, values := range c.params {
+ if shouldCopyParam(key, excludeParams, reserveParams) {
+ copied := make([]string, len(values))
+ copy(copied, values)
+ params[key] = copied
+ }
}
- return newURL
+ return params
+}
+
+func paramsCapacityForClone(paramsLen int, excludeParams *gxset.HashSet,
reserveParams []string) int {
+ if len(reserveParams) > 0 && len(reserveParams) < paramsLen {
+ paramsLen = len(reserveParams)
+ }
+ if excludeParams != nil {
+ paramsLen -= excludeParams.Size()
+ }
+ if paramsLen < 0 {
+ return 0
+ }
+ return paramsLen
+}
+
+func shouldCopyParam(key string, excludeParams *gxset.HashSet, reserveParams
[]string) bool {
+ if excludeParams != nil && excludeParams.Contains(key) {
+ return false
+ }
+ return len(reserveParams) == 0 || slices.Contains(reserveParams, key)
}
// Clone will copy the URL
diff --git a/common/url_test.go b/common/url_test.go
index 9e56e22ac..1394914f6 100644
--- a/common/url_test.go
+++ b/common/url_test.go
@@ -1075,6 +1075,29 @@ func TestCloneWithParams(t *testing.T) {
assert.Equal(t, []string{"method1", "method2"}, cloned.Methods)
}
+func TestCloneWithFilterPreservesMultiValueParams(t *testing.T) {
+ u := &URL{}
+ u.SetParams(url.Values{
+ "group": {"a", "b"},
+ "drop": {"drop-value"},
+ "other": {"other-value"},
+ })
+
+ excludeSet := gxset.NewSet("drop")
+ cloned := u.CloneWithFilter(excludeSet, []string{"group", "drop"})
+
+ clonedParams := cloned.GetParams()
+ assert.Equal(t, []string{"a", "b"}, clonedParams["group"])
+ assert.NotContains(t, clonedParams, "drop")
+ assert.NotContains(t, clonedParams, "other")
+
+ cloned.paramsLock.Lock()
+ cloned.params["group"][0] = "changed"
+ cloned.paramsLock.Unlock()
+
+ assert.Equal(t, []string{"a", "b"}, u.GetParams()["group"])
+}
+
func TestURLCompare(t *testing.T) {
u1, _ := NewURL("dubbo://127.0.0.1:20000/a.Service")
u2, _ := NewURL("dubbo://127.0.0.1:20000/b.Service")
@@ -1359,6 +1382,47 @@ func TestMergeURLWithMethodParams(t *testing.T) {
assert.Equal(t, "5000",
mergedUrl.GetParam("methods.testMethod."+constant.TimeoutKey, ""))
}
+func TestMergeURLPreservesMultiValueParams(t *testing.T) {
+ local := &URL{}
+ local.SetParams(url.Values{
+ "group": {"local-a", "local-b"},
+ })
+
+ remote := &URL{}
+ remote.SetParams(url.Values{
+ "group": {"remote-a", "remote-b"},
+ "version": {"remote-v1", "remote-v2"},
+ })
+
+ merged := local.MergeURL(remote)
+ mergedParams := merged.GetParams()
+
+ assert.Equal(t, []string{"local-a", "local-b"}, mergedParams["group"])
+ assert.Equal(t, []string{"remote-v1", "remote-v2"},
mergedParams["version"])
+
+ remote.paramsLock.Lock()
+ remote.params["version"][0] = "changed"
+ remote.paramsLock.Unlock()
+
+ assert.Equal(t, []string{"remote-v1", "remote-v2"},
merged.GetParams()["version"])
+}
+
+func TestMergeURLUsesNonDefaultParamSemantics(t *testing.T) {
+ local := &URL{}
+ local.SetParams(url.Values{
+ "group": {"", "local-b"},
+ })
+
+ remote := &URL{}
+ remote.SetParams(url.Values{
+ "group": {"remote-a", "remote-b"},
+ })
+
+ merged := local.MergeURL(remote)
+
+ assert.Equal(t, []string{"remote-a", "remote-b"},
merged.GetParams()["group"])
+}
+
func TestURLWithPathAlreadyHasSlash(t *testing.T) {
u := NewURLWithOptions(WithPath("/com.test.Service"))
assert.Equal(t, "/com.test.Service", u.Path)
