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