sodonnel commented on code in PR #11884:
URL: https://github.com/apache/ozone/pull/11884#discussion_r4454708552


##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ECReplicationConfig.java:
##########
@@ -67,6 +102,50 @@ public static String allValuesAsString() {
     }
   }
 
+  /** Same as {@link #ECReplicationConfig(String)}, but returns a shared 
instance for the predefined configs. */
+  public static ECReplicationConfig getInstance(String string) {
+    return parse(string);
+  }
+
+  /** Same as the proto constructor, but returns a shared instance for the 
predefined configs, without creating one. */
+  public static ECReplicationConfig getInstance(HddsProtos.ECReplicationConfig 
proto) {
+    final EcCodec codec = getCodec(proto.getCodec());
+    if (codec == null) {
+      return new ECReplicationConfig(proto); // throws the same exception as 
before
+    }
+    final ECReplicationConfig predefined = getPredefined(proto.getData(), 
proto.getParity(), codec,
+        proto.getEcChunkSize());
+    return predefined != null ? predefined

Review Comment:
   In the event that we don't find a predefined one, we just create a new 
instance and don't add it to the pre-defined set.
   
   For me there is a lot of "stuff" happening in this class iterating over 
schemes and chunk sizes etc, which makes it rather complex. Is there some way 
we can just memoise the results of get instance so it creates a new object and 
then caches it against a key? There is also a case where we parse the EC string 
below and it just looks up a map based on the string key.
   
   I think the goal is to have many less EC objects in memory, so even if we 
pay a little bit for a simpler lookup mechanism that handle all instances (not 
just predefined) it may be good enough.



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