From 6d818dcbe5187d15a46cced7893be28df1cec725 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Tue, 27 Aug 2019 16:04:55 +0200 Subject: [PATCH] Do not retain leaf nodes in containers by default Leaf nodes are observed to dominate our memory footprint. This patch modifies immutable implementations of NormalizedNodeContainers such that they eliminate any leaf nodes when they are stored, retaining only the encapsulated value. Leaf nodes are then re-created on access as needed. Note this changes two aspects of operation: 1) looking up a leaf will yield a new object every time it is invoked, hence callers must not rely on on returned objects to be identical. 2) getValue() is operating on the backing map's entrySet() and requires its transformation. This means that multiple iterations over values will not necessarily yield same objects. Neither of these violates effective immutability contract of NormalizeNode, as the returned objects will compare as equal. This behavior can be switched off at runtime by setting the system property: org.opendaylight.yangtools.yang.data.impl.schema.nodes.lazy-leaves=false JIRA: YANGTOOLS-1019 Change-Id: I00fc3ac0b64290068e8a6e4c8972454729fa9011 Signed-off-by: Robert Varga --- ...ractImmutableDataContainerNodeBuilder.java | 18 ++- .../ImmutableAugmentationNodeBuilder.java | 2 +- .../impl/ImmutableChoiceNodeBuilder.java | 4 +- .../impl/ImmutableContainerNodeBuilder.java | 4 +- .../impl/ImmutableMapEntryNodeBuilder.java | 3 +- .../ImmutableUnkeyedListEntryNodeBuilder.java | 4 +- ...ImmutableYangModeledAnyXmlNodeBuilder.java | 4 +- .../AbstractImmutableDataContainerNode.java | 17 +-- .../impl/schema/nodes/LazyLeafOperations.java | 120 ++++++++++++++++++ .../data/jaxen/DerefXPathFunctionTest.java | 7 +- 10 files changed, 145 insertions(+), 38 deletions(-) create mode 100644 yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/LazyLeafOperations.java diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/AbstractImmutableDataContainerNodeBuilder.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/AbstractImmutableDataContainerNodeBuilder.java index f442d673a7..dc8cf1e33c 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/AbstractImmutableDataContainerNodeBuilder.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/AbstractImmutableDataContainerNodeBuilder.java @@ -21,6 +21,7 @@ import org.opendaylight.yangtools.yang.data.impl.schema.builder.api.DataContaine import org.opendaylight.yangtools.yang.data.impl.schema.builder.api.NormalizedNodeContainerBuilder; import org.opendaylight.yangtools.yang.data.impl.schema.nodes.AbstractImmutableDataContainerNode; import org.opendaylight.yangtools.yang.data.impl.schema.nodes.CloneableMap; +import org.opendaylight.yangtools.yang.data.impl.schema.nodes.LazyLeafOperations; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -42,7 +43,7 @@ abstract class AbstractImmutableDataContainerNodeBuilder> value; + private Map value; private I nodeIdentifier; /* @@ -86,13 +87,12 @@ abstract class AbstractImmutableDataContainerNodeBuilder getChild(final PathArgument child) { - return value.get(child); + return LazyLeafOperations.getChild(value, child); } - protected final Map> buildValue() { + protected final Map buildValue() { if (value instanceof ModifiableMapPhase) { - return ((ModifiableMapPhase>)value) - .toUnmodifiableMap(); + return ((ModifiableMapPhase)value).toUnmodifiableMap(); } dirty = true; @@ -102,11 +102,9 @@ abstract class AbstractImmutableDataContainerNodeBuilder>) value) - .toModifiableMap(); + value = ((UnmodifiableMapPhase) value).toModifiableMap(); } else if (value instanceof CloneableMap) { - value = ((CloneableMap>) value) - .createMutableClone(); + value = ((CloneableMap) value).createMutableClone(); } else { value = newHashMap(value); } @@ -127,7 +125,7 @@ abstract class AbstractImmutableDataContainerNodeBuilder withChild(final DataContainerChild child) { checkDirty(); - this.value.put(child.getIdentifier(), child); + LazyLeafOperations.putChild(value, child); return this; } diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableAugmentationNodeBuilder.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableAugmentationNodeBuilder.java index 8f4357d24c..912cdc487a 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableAugmentationNodeBuilder.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableAugmentationNodeBuilder.java @@ -74,7 +74,7 @@ public class ImmutableAugmentationNodeBuilder extends AbstractImmutableDataContainerNode implements AugmentationNode { ImmutableAugmentationNode(final AugmentationIdentifier nodeIdentifier, - final Map> children) { + final Map children) { super(children, nodeIdentifier); } } diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableChoiceNodeBuilder.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableChoiceNodeBuilder.java index 61d14c614d..80be092d32 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableChoiceNodeBuilder.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableChoiceNodeBuilder.java @@ -12,7 +12,6 @@ import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument; import org.opendaylight.yangtools.yang.data.api.schema.ChoiceNode; -import org.opendaylight.yangtools.yang.data.api.schema.DataContainerChild; import org.opendaylight.yangtools.yang.data.impl.schema.builder.api.DataContainerNodeBuilder; import org.opendaylight.yangtools.yang.data.impl.schema.nodes.AbstractImmutableDataContainerNode; @@ -54,8 +53,7 @@ public class ImmutableChoiceNodeBuilder extends AbstractImmutableDataContainerNo private static final class ImmutableChoiceNode extends AbstractImmutableDataContainerNode implements ChoiceNode { - ImmutableChoiceNode(final NodeIdentifier nodeIdentifier, - final Map> children) { + ImmutableChoiceNode(final NodeIdentifier nodeIdentifier, final Map children) { super(children, nodeIdentifier); } } diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableContainerNodeBuilder.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableContainerNodeBuilder.java index ec821098a6..caa1e7d080 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableContainerNodeBuilder.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableContainerNodeBuilder.java @@ -12,7 +12,6 @@ import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument; import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode; -import org.opendaylight.yangtools.yang.data.api.schema.DataContainerChild; import org.opendaylight.yangtools.yang.data.impl.schema.builder.api.DataContainerNodeBuilder; import org.opendaylight.yangtools.yang.data.impl.schema.nodes.AbstractImmutableDataContainerNode; @@ -55,8 +54,7 @@ public class ImmutableContainerNodeBuilder protected static final class ImmutableContainerNode extends AbstractImmutableDataContainerNode implements ContainerNode { - ImmutableContainerNode(final NodeIdentifier nodeIdentifier, - final Map> children) { + ImmutableContainerNode(final NodeIdentifier nodeIdentifier, final Map children) { super(children, nodeIdentifier); } } diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableMapEntryNodeBuilder.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableMapEntryNodeBuilder.java index 6e6b7a59d4..33fd8a8ef1 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableMapEntryNodeBuilder.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableMapEntryNodeBuilder.java @@ -78,7 +78,6 @@ public class ImmutableMapEntryNodeBuilder } } - @Override public DataContainerNodeBuilder withValue( final Collection> withValue) { @@ -124,7 +123,7 @@ public class ImmutableMapEntryNodeBuilder extends AbstractImmutableDataContainerNode implements MapEntryNode { ImmutableMapEntryNode(final NodeIdentifierWithPredicates nodeIdentifier, - final Map> children) { + final Map children) { super(children, nodeIdentifier); } } diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableUnkeyedListEntryNodeBuilder.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableUnkeyedListEntryNodeBuilder.java index 599789a517..93823e871e 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableUnkeyedListEntryNodeBuilder.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableUnkeyedListEntryNodeBuilder.java @@ -11,7 +11,6 @@ import java.util.Map; import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument; -import org.opendaylight.yangtools.yang.data.api.schema.DataContainerChild; import org.opendaylight.yangtools.yang.data.api.schema.UnkeyedListEntryNode; import org.opendaylight.yangtools.yang.data.impl.schema.builder.api.DataContainerNodeBuilder; import org.opendaylight.yangtools.yang.data.impl.schema.nodes.AbstractImmutableDataContainerNode; @@ -56,8 +55,7 @@ public class ImmutableUnkeyedListEntryNodeBuilder protected static final class ImmutableUnkeyedListEntryNode extends AbstractImmutableDataContainerNode implements UnkeyedListEntryNode { - ImmutableUnkeyedListEntryNode(final NodeIdentifier nodeIdentifier, - final Map> children) { + ImmutableUnkeyedListEntryNode(final NodeIdentifier nodeIdentifier, final Map children) { super(children, nodeIdentifier); } } diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableYangModeledAnyXmlNodeBuilder.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableYangModeledAnyXmlNodeBuilder.java index 2916a50a2c..3abeea8220 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableYangModeledAnyXmlNodeBuilder.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/builder/impl/ImmutableYangModeledAnyXmlNodeBuilder.java @@ -14,7 +14,6 @@ import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.yangtools.odlext.model.api.YangModeledAnyXmlSchemaNode; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument; -import org.opendaylight.yangtools.yang.data.api.schema.DataContainerChild; import org.opendaylight.yangtools.yang.data.api.schema.YangModeledAnyXmlNode; import org.opendaylight.yangtools.yang.data.impl.schema.builder.api.DataContainerNodeBuilder; import org.opendaylight.yangtools.yang.data.impl.schema.nodes.AbstractImmutableDataContainerNode; @@ -59,8 +58,7 @@ public final class ImmutableYangModeledAnyXmlNodeBuilder extends private final @NonNull ContainerSchemaNode contentSchema; - ImmutableYangModeledAnyXmlNode(final NodeIdentifier nodeIdentifier, - final Map> value, + ImmutableYangModeledAnyXmlNode(final NodeIdentifier nodeIdentifier, final Map value, final ContainerSchemaNode contentSchema) { super(value, nodeIdentifier); this.contentSchema = requireNonNull(contentSchema, "Schema of yang modeled anyXml content cannot be null."); diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/AbstractImmutableDataContainerNode.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/AbstractImmutableDataContainerNode.java index 231a703e3c..dabcc5777e 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/AbstractImmutableDataContainerNode.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/AbstractImmutableDataContainerNode.java @@ -18,10 +18,9 @@ import org.opendaylight.yangtools.yang.data.api.schema.DataContainerNode; public abstract class AbstractImmutableDataContainerNode extends AbstractImmutableNormalizedNode>> implements DataContainerNode { - private final Map> children; + private final Map children; - public AbstractImmutableDataContainerNode( - final Map> children, final K nodeIdentifier) { + protected AbstractImmutableDataContainerNode(final Map children, final K nodeIdentifier) { super(nodeIdentifier); this.children = ImmutableOffsetMap.unorderedCopyOf(children); @@ -29,12 +28,12 @@ public abstract class AbstractImmutableDataContainerNode @Override public final Optional> getChild(final PathArgument child) { - return Optional.ofNullable(children.get(child)); + return LazyLeafOperations.findChild(children, child); } @Override public final Collection> getValue() { - return children.values(); + return LazyLeafOperations.getValue(children); } @Override @@ -46,13 +45,12 @@ public abstract class AbstractImmutableDataContainerNode * DO NOT USE THIS METHOD. * *

- * This is an implementation-internal API and no outside users should use it. If you do, - * you are asking for trouble, as the returned object is not guaranteed to conform to - * java.util.Map interface. + * This is an implementation-internal API and no outside users should use it. If you do, you are asking for trouble, + * as the returned object is not guaranteed to conform to java.util.Map interface, nor is its contents well-defined. * * @return An unmodifiable view if this node's children. */ - public final Map> getChildren() { + public final Map getChildren() { return children; } @@ -60,6 +58,5 @@ public abstract class AbstractImmutableDataContainerNode protected boolean valueEquals(final AbstractImmutableNormalizedNode other) { return other instanceof AbstractImmutableDataContainerNode && children.equals( ((AbstractImmutableDataContainerNode) other).children); - } } diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/LazyLeafOperations.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/LazyLeafOperations.java new file mode 100644 index 0000000000..868b014cb3 --- /dev/null +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/nodes/LazyLeafOperations.java @@ -0,0 +1,120 @@ +/* + * Copyright (c) 2019 PANTHEON.tech, s.r.o. and others. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v1.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v10.html + */ +package org.opendaylight.yangtools.yang.data.impl.schema.nodes; + +import static com.google.common.base.Verify.verify; +import static java.util.Objects.requireNonNull; + +import com.google.common.annotations.Beta; +import com.google.common.collect.Collections2; +import java.util.Collection; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Optional; +import org.eclipse.jdt.annotation.NonNull; +import org.eclipse.jdt.annotation.Nullable; +import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier; +import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument; +import org.opendaylight.yangtools.yang.data.api.schema.DataContainerChild; +import org.opendaylight.yangtools.yang.data.api.schema.LeafNode; +import org.opendaylight.yangtools.yang.data.api.schema.LeafSetEntryNode; +import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Support utilities for dealing with Maps which would normally hold {@link DataContainerChild} values, but are modified + * to eliminate {@link LeafNode} instances. + * + *

+ * This class holds implementation logic which controls lifecycle of {@link LeafNode}s by providing a central policy + * point for how the implementation treats these nodes. There are two modes of operation: + *

    + *
  • eager, which means leaf nodes are retained by their parent
  • + *
  • lazy, which means leaf nodes are created whenever they are queried and no attempt is made to retain them
    • + *
+ * + *

+ * Selection of the mode in effect is available through {@value #EXPENDABLE_PROP_NAME} system property. + */ +@Beta +public final class LazyLeafOperations { + private static final Logger LOG = LoggerFactory.getLogger(LazyLeafOperations.class); + private static final String EXPENDABLE_PROP_NAME = + "org.opendaylight.yangtools.yang.data.impl.schema.nodes.lazy-leaves"; + + /** + * Global enabled run-time constant. If set to true, this class will treat {@link LeafNode} and + * {@link LeafSetEntryNode} as an expendable object. This constant is controlled by {@value #EXPENDABLE_PROP_NAME} + * system property. + */ + private static final boolean EXPENDABLE; + + static { + EXPENDABLE = Boolean.parseBoolean(System.getProperty(EXPENDABLE_PROP_NAME, "true")); + LOG.info("Leaf nodes are treated as {} nodes", EXPENDABLE ? "transient" : "regular"); + } + + private LazyLeafOperations() { + + } + + public static Optional> findChild(final Map map, + final PathArgument key) { + final Object value = map.get(key); + return value == null ? Optional.empty() : Optional.of(decodeChild(key, value)); + } + + public static @Nullable DataContainerChild getChild(final Map map, + final PathArgument key) { + final Object value = map.get(key); + return value == null ? null : decodeChild(key, value); + } + + public static void putChild(final Map map, final DataContainerChild child) { + final DataContainerChild node = requireNonNull(child); + map.put(child.getIdentifier(), EXPENDABLE ? encodeExpendableChild(node) : node); + } + + @SuppressWarnings({ "rawtypes", "unchecked" }) + public static @NonNull Collection> getValue(final Map map) { + return EXPENDABLE ? Collections2.transform(map.entrySet(), LazyLeafOperations::decodeEntry) + // This is an ugly cast, but it is accurate IFF all modifications are done through this class + : (Collection) map.values(); + } + + private static @NonNull Object encodeExpendableChild(final DataContainerChild key) { + return key instanceof LeafNode ? ((LeafNode) key).getValue() : requireNonNull(key); + } + + private static @Nullable DataContainerChild decodeChild(final PathArgument key, final @NonNull Object value) { + return EXPENDABLE ? decodeExpendableChild(key, value) : verifyCast(value); + } + + private static @NonNull DataContainerChild decodeExpendableChild(final PathArgument key, + @NonNull final Object value) { + return value instanceof DataContainerChild ? (DataContainerChild) value : coerceLeaf(key, value); + } + + private static @NonNull DataContainerChild verifyCast(final @NonNull Object value) { + verify(value instanceof DataContainerChild, "Unexpected child %s", value); + return (DataContainerChild)value; + } + + private static @NonNull DataContainerChild decodeEntry( + final @NonNull Entry entry) { + final Object value = entry.getValue(); + return value instanceof DataContainerChild ? (DataContainerChild) value + : coerceLeaf(entry.getKey(), value); + } + + private static @NonNull LeafNode coerceLeaf(final PathArgument key, final Object value) { + verify(key instanceof NodeIdentifier, "Unexpected value %s for child %s", value, key); + return ImmutableNodes.leafNode((NodeIdentifier) key, value); + } +} diff --git a/yang/yang-data-jaxen/src/test/java/org/opendaylight/yangtools/yang/data/jaxen/DerefXPathFunctionTest.java b/yang/yang-data-jaxen/src/test/java/org/opendaylight/yangtools/yang/data/jaxen/DerefXPathFunctionTest.java index 24a08803a1..e383694cc7 100644 --- a/yang/yang-data-jaxen/src/test/java/org/opendaylight/yangtools/yang/data/jaxen/DerefXPathFunctionTest.java +++ b/yang/yang-data-jaxen/src/test/java/org/opendaylight/yangtools/yang/data/jaxen/DerefXPathFunctionTest.java @@ -8,6 +8,7 @@ package org.opendaylight.yangtools.yang.data.jaxen; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; @@ -87,7 +88,7 @@ public class DerefXPathFunctionTest { final Object derefResult = derefFunction.call(normalizedNodeContext, ImmutableList.of()); assertNotNull(derefResult); assertTrue(derefResult instanceof NormalizedNode); - assertSame(referencedLeafNode, derefResult); + assertEquals(referencedLeafNode, derefResult); } @Test @@ -120,7 +121,7 @@ public class DerefXPathFunctionTest { Object derefResult = derefFunction.call(normalizedNodeContext, ImmutableList.of()); assertNotNull(derefResult); assertTrue(derefResult instanceof NormalizedNode); - assertSame(referencedLeafNode, derefResult); + assertEquals(referencedLeafNode, derefResult); final YangInstanceIdentifier relLeafrefPath = YangInstanceIdentifier.of(MY_INNER_CONTAINER) .node(REL_LEAFREF_LEAF); @@ -129,7 +130,7 @@ public class DerefXPathFunctionTest { derefResult = derefFunction.call(normalizedNodeContext, ImmutableList.of()); assertNotNull(derefResult); assertTrue(derefResult instanceof NormalizedNode); - assertSame(referencedLeafNode, derefResult); + assertEquals(referencedLeafNode, derefResult); } @Test -- 2.36.6