No-SilverBullet commented on PR #2799: URL: https://github.com/apache/dubbo-go/pull/2799#issuecomment-2729866351
> @No-SilverBullet @FoghostCn I updated URL.Clone() in common/url.go to use defer for lock releases and added nil checks for params and attributes as per the suggestions. Additionally, I enhanced TestSubURLCopy to test Protocol and params for thorough deep-copy validation and fixed TestCompareURLEqualFunc with SetCompareURLEqualFunc(defaultCompareURLEqual) resets to address state pollution. 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. Thanks -- 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-unsubscr...@dubbo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: notifications-unsubscr...@dubbo.apache.org For additional commands, e-mail: notifications-h...@dubbo.apache.org
