blerer commented on code in PR #15457:
URL: https://github.com/apache/lucene/pull/15457#discussion_r2578451987


##########
lucene/core/src/java/org/apache/lucene/index/FieldInfo.java:
##########
@@ -675,7 +674,7 @@ public synchronized String putAttribute(String key, String 
value) {
     String oldValue = newMap.put(key, value);
     // This needs to be thread-safe as multiple threads may be updating 
(different) attributes
     // concurrently due to concurrent merging.
-    attributes = Collections.unmodifiableMap(newMap);
+    attributes = Map.copyOf(newMap);

Review Comment:
   It  is a good point. Effectively this method is often called twice in a row 
within some `DocValuesProducer` and in some loop within `FieldInfos.Builder` 
(which seems to be called during merges). So yes we should probably keep 
`unmodifiableMap` here. It would also probably make sense to have some 
`putAttributes` methods that perform the update in one go instead of creating a 
new Map and reacquiring the synchronize lock each time. What do you think?



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to