Fanoid commented on code in PR #250:
URL: https://github.com/apache/flink-ml/pull/250#discussion_r1305396094


##########
flink-ml-iteration/flink-ml-iteration-common/src/main/java/org/apache/flink/iteration/datacache/nonkeyed/OperatorScopeManagedMemoryManager.java:
##########
@@ -18,33 +18,75 @@
 
 package org.apache.flink.iteration.datacache.nonkeyed;
 
+import org.apache.flink.annotation.PublicEvolving;
+import org.apache.flink.runtime.jobgraph.OperatorID;
 import org.apache.flink.util.Preconditions;
 
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.WeakHashMap;
 
 /**
  * A manager for operator-scope managed memory.
  *
- * <p>Every operator must create one (and only one) instance of this class to 
manage usages of
- * operator-scope managed memery. In the operator, for every usage, {@link 
#register} is called to
- * declare its weight of memory usage with a key. Then, the fraction of memory 
can be obtained by
- * calling {@link #getFraction}.
+ * <p>Every operator must call {@link #getOrCreate} to get an instance to 
manage usages of
+ * operator-scope managed memory.
  *
- * <p>Note that all calls of {@link #register} must be before those of {@link 
#getFraction}.
+ * <p>In the operator, for every usage of operator-scope managed memory, 
{@link #register} is called
+ * to declare its weight of memory usage with a key. Then, the fraction of 
memory can be obtained by
+ * calling {@link #getFraction}. Note that all calls of {@link #register} must 
be before those of
+ * {@link #getFraction}.
  */
+@PublicEvolving
 public class OperatorScopeManagedMemoryManager {
 
+    /**
+     * Stores instances corresponding to operators. The instance is expected 
to be released after
+     * some point after the corresponding operator ID is unused.
+     */
+    private static final Map<OperatorID, OperatorScopeManagedMemoryManager> 
managers =
+            Collections.synchronizedMap(new WeakHashMap<>());
+
+    /** Stores keys and weights of memory usages. */
     protected Map<String, Double> weights = new HashMap<>();
+    /** Indicates whether the `weights` is frozen. */
     protected boolean frozen = false;
+    /** Stores sum of weights of all usages. */
     protected double sum;
 
+    OperatorScopeManagedMemoryManager() {}

Review Comment:
   The empty constructor is actually used to limit its access. But I think 
using `private` scope is more reasonable here. 
   Please check the changes.



-- 
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-unsubscr...@flink.apache.org

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

Reply via email to