[GitHub] [avro] clesaec commented on pull request #2090: [AVRO-3713]: [Java] Fix Map synchronization regression

["clesaec (via GitHub)" <gi...@apache.org>]

clesaec commented on PR #2090:
URL: https://github.com/apache/avro/pull/2090#issuecomment-1428000122

   Yes, sorry, i put LGTM may be little fast, as i was agree with usage of "computeIfAbsent is OK, but indeed, Weak keys are useful here, because the Map is owned by a Singleton, so, even if a field instance goes out of the scope, it wouldn't be garbaged because of this Map.
   "Threadsafe weak keys map implementation " : No i don't know any one, but as it's a private field only used by one method, and value is only set once, there is may be another way, like 
   
   `private final Map<Field, Object> defaultValueCache = new WeakHashMap<>();`
   and
   ```java
       final Object defaultValue;
       if (!defaultValueCache.containsKey(field)) {
         defaultValue = ... calculate value here ... 
         synchronized (this.defaultValueCache) {
            this.defaultValueCache.put(field, defaultValue);
         }
       }
       else {
           defaultValue = defaultValueCache.get(field);
       }
   ```
   Map without "delete", don't have to be synchronized on "get" ?
   
   
   


-- 
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: issues-unsubscribe@avro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org