Replace copier.Copy with optimized manual deep copy in URL.Clone (#2678)#2799
Replace copier.Copy with optimized manual deep copy in URL.Clone (#2678)#2799Nexusrex18 wants to merge 4 commits intoapache:developfrom
Conversation
common/url.go
Outdated
| Methods: make([]string, len(c.Methods)), | ||
| } | ||
| copy(newURL.Methods, c.Methods) | ||
| newURL.params = make(url.Values, len(c.params)) |
There was a problem hiding this comment.
put this line of code in the newURL initialization above, more concise
common/url.go
Outdated
| Path: c.Path, | ||
| Username: c.Username, | ||
| Password: c.Password, | ||
| Methods: make([]string, len(c.Methods)), |
There was a problem hiding this comment.
change to -> Methods: make([]string, len(c.Methods)) , and remove line843 copy, more concise
|
@No-SilverBullet @FoghostCn |
|
have changed the target branch from main to develop. |
Sry for the late response, LGTM, but we need to check the CI error. And can you update the new benchmark? The performance may be reduced after the lock adjustment (the lock holding time becomes longer), but for the sake of code readability, I want to know whether this performance loss is acceptable. Besides, since you added the nil check for params and attributes, It would be better to move the initialization inside the nil check to avoid unnecessary initialization. like this
|
|
@No-SilverBullet |
|
Have you pull the newest upsteam/develop branch? CI error still exists. |
Signed-off-by: Nexusrex18 <lavisnj350@gmail.com>
|
@No-SilverBullet even after fetching the latest changes , getting the same issue so should i try any other approach or recreate a new pull request with same changes ? |
|



Fixes by replacing
copier.Copywith a manual deep copy inURL.Clone(), optimized with pre-allocated maps forparamsandattributes. This reduces memory and CPU overhead, especially for large data volumes.Key changes:
copier.Copywith manual implementation inURL.Clone().github.com/jinzhu/copierimport fromurl.goand dependency fromgo.modviago mod tidy.TestSubURLCopyto verify deep-copy behavior.BenchmarkCloneto test the new implementation.Benchmarks:
The new implementation is faster (~1.6x at 1,000 entries, ~2x at 100), uses less memory (~14% at 1,000, ~40% at 100), and reduces allocations (~12% at 1,000, ~50% at 100).
Note: Unrelated test failures in
rpc_service_test.go(e.g., unexportedtestService) were observed but pre-exist this change and are outside this PR’s scope.Fixes: #2678
Tested with:
go test -v -run TestSubURLCopy: Passedgo test -bench=BenchmarkClone -benchmem: Confirmed performance