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]