Copilot commented on code in PR #12540:
URL: https://github.com/apache/maven/pull/12540#discussion_r3650097107


##########
impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultModelUrlNormalizer.java:
##########
@@ -46,12 +46,11 @@ public DefaultModelUrlNormalizer(UrlNormalizer 
urlNormalizer) {
     }
 
     @Override
-    public Model normalize(Model model, ModelBuilderRequest request) {
+    public void normalize(Model model, Model.Builder builder, 
ModelBuilderRequest request) {
         if (model == null) {
-            return null;
+            return;
         }
 
-        Model.Builder builder = Model.newBuilder(model);
         builder.url(normalize(model.getUrl()));

Review Comment:
   `normalize(model.getUrl())` is applied unconditionally and then written to 
the builder. If the normalizer returns a new String instance with identical 
content (common when no "../" needs removal), setting it will defeat 
`Builder#build()`'s identity-based short-circuit and force an unnecessary new 
Model allocation. Guard the setter so it only runs when the normalized value is 
actually different (including value-equality).
   
   This issue also appears in the following locations of the same file:
   - line 56
   - line 65



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to