From: Robert Varga Date: Tue, 5 May 2020 20:03:26 +0000 (+0200) Subject: Add lazily-instantiated lists X-Git-Tag: v6.0.1~34 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=66d31028a61921c152ed46f46dd6dee3574af9f7;p=mdsal.git Add lazily-instantiated lists Lists are usually searched linearly. For large lists we can end up instantiating a large number of items upfront, some of which is wasteful. The cutoff point can be configured via a JVM property, which is interpreted as a dynamic constant. JIRA: MDSAL-551 Change-Id: I9e7c987929481143b931e763f4ef0313745a5c89 Signed-off-by: Robert Varga --- diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/DataObjectCodecContext.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/DataObjectCodecContext.java index 82e1ce5913..b1083ffa0b 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/DataObjectCodecContext.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/DataObjectCodecContext.java @@ -87,6 +87,7 @@ public abstract class DataObjectCodecContext, DataContainerCodecPrototype> byStreamClass; private final ImmutableMap, DataContainerCodecPrototype> byBindingArgClass; private final ImmutableMap possibleAugmentations; + private final @NonNull Class> generatedClass; private final MethodHandle proxyConstructor; // FIXME: the presence of these two volatile fields may be preventing us from being able to improve @@ -158,7 +159,6 @@ public abstract class DataObjectCodecContext> generatedClass; if (Augmentable.class.isAssignableFrom(bindingClass)) { this.possibleAugmentations = factory().getRuntimeContext().getAvailableAugmentationTypes(getSchema()); generatedClass = CodecDataObjectGenerator.generateAugmentable(prototype.getFactory().getLoader(), @@ -541,6 +541,10 @@ public abstract class DataObjectCodecContext> generatedClass() { + return generatedClass; + } + @Override public InstanceIdentifier.PathArgument deserializePathArgument(final YangInstanceIdentifier.PathArgument arg) { checkArgument(getDomPathArgument().equals(arg)); diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/KeyedListNodeCodecContext.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/KeyedListNodeCodecContext.java index 44151a7b81..206baa7c15 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/KeyedListNodeCodecContext.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/KeyedListNodeCodecContext.java @@ -14,6 +14,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap.Builder; import java.lang.reflect.Method; import java.util.List; +import java.util.Map; import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.yangtools.yang.binding.DataObject; import org.opendaylight.yangtools.yang.binding.Identifiable; @@ -41,11 +42,11 @@ abstract class KeyedListNodeCodecContext> } @Override - Object fromMap(final MapNode map, final int size) { + Map fromMap(final MapNode map, final int size) { // FIXME: MDSAL-539: Make this a lazily-populated map final Builder builder = ImmutableMap.builderWithExpectedSize(size); for (MapEntryNode node : map.getValue()) { - final D entry = fromMapEntry(node); + final D entry = createBindingProxy(node); builder.put(entry.key(), entry); } return builder.build(); diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/LazyBindingList.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/LazyBindingList.java new file mode 100644 index 0000000000..c0f3fcb7bb --- /dev/null +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/LazyBindingList.java @@ -0,0 +1,165 @@ +/* + * Copyright (c) 2020 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.mdsal.binding.dom.codec.impl; + +import static com.google.common.base.Verify.verify; +import static java.util.Objects.requireNonNull; + +import com.google.common.annotations.VisibleForTesting; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.VarHandle; +import java.util.AbstractList; +import java.util.Collection; +import java.util.Comparator; +import java.util.List; +import java.util.RandomAccess; +import java.util.function.UnaryOperator; +import org.eclipse.jdt.annotation.NonNull; +import org.opendaylight.yangtools.concepts.Immutable; +import org.opendaylight.yangtools.yang.binding.DataObject; +import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNodeContainer; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Lazily-populated List implementation backed by NormalizedNodes. This implementation defers creating Binding objects + * until they are actually needed, caching them in a pre-allocated array. + * + *

+ * The cost of this deferred instantiation is two-fold: + *

    + *
  • each access issues a {@link VarHandle#getAcquire(Object...)} load and a class equality check
    • + *
  • initial load additionally incurs a {@link VarHandle#compareAndExchangeRelease(Object...)} store
    • + *
+ * + * @param the type of elements in this list + */ +final class LazyBindingList extends AbstractList implements Immutable, RandomAccess { + private static final VarHandle OBJ_AA = MethodHandles.arrayElementVarHandle(Object[].class); + private static final Logger LOG = LoggerFactory.getLogger(LazyBindingList.class); + private static final String LAZY_CUTOFF_PROPERTY = + "org.opendaylight.mdsal.binding.dom.codec.impl.LazyBindingList.max-eager-elements"; + private static final int DEFAULT_LAZY_CUTOFF = 16; + + @VisibleForTesting + static final int LAZY_CUTOFF; + + static { + final int value = Integer.getInteger(LAZY_CUTOFF_PROPERTY, DEFAULT_LAZY_CUTOFF); + if (value < 0) { + LOG.info("Lazy population of lists disabled"); + LAZY_CUTOFF = Integer.MAX_VALUE; + } else { + LOG.info("Using lazy population for lists larger than {} element(s)", value); + LAZY_CUTOFF = value; + } + } + + private final ListNodeCodecContext codec; + private final Object[] objects; + + private LazyBindingList(final ListNodeCodecContext codec, + final Collection> entries) { + this.codec = requireNonNull(codec); + objects = entries.toArray(); + } + + static @NonNull List create(final ListNodeCodecContext codec, final int size, + final Collection> entries) { + if (size == 1) { + // Do not bother with lazy instantiation in case of a singleton + return List.of(codec.createBindingProxy(entries.iterator().next())); + } + return size > LAZY_CUTOFF ? new LazyBindingList<>(codec, entries) : eagerList(codec, size, entries); + } + + private static @NonNull List eagerList(final ListNodeCodecContext codec, + final int size, final Collection> entries) { + @SuppressWarnings("unchecked") + final E[] objs = (E[]) new DataObject[size]; + int offset = 0; + for (NormalizedNodeContainer node : entries) { + objs[offset++] = codec.createBindingProxy(node); + } + verify(offset == objs.length); + return List.of(objs); + } + + @Override + public int size() { + return objects.length; + } + + @Override + public E get(final int index) { + final Object obj = OBJ_AA.getAcquire(objects, index); + // Check whether the object has been converted. The object is always non-null, but it can either be in DOM form + // (either a MapEntryNode or UnkeyedListEntryNode) or in Binding form. We know the exact class for the latter, + // as we are creating it via codec -- hence we can perform a direct comparison. + // + // We could do a Class.isInstance() check here, but since the implementation is not marked as final (yet) we + // would be at the mercy of CHA being able to prove this invariant. + return obj.getClass() == codec.generatedClass() ? (E) obj : load(index, (NormalizedNodeContainer) obj); + } + + private @NonNull E load(final int index, final NormalizedNodeContainer node) { + final E ret = codec.createBindingProxy(node); + final Object witness; + return (witness = OBJ_AA.compareAndExchangeRelease(objects, index, node, ret)) == node ? ret : (E) witness; + } + + @Override + @SuppressWarnings("checkstyle:parameterName") + public boolean remove(final Object o) { + throw uoe(); + } + + @Override + @SuppressWarnings("checkstyle:parameterName") + public boolean addAll(final Collection c) { + throw uoe(); + } + + @Override + @SuppressWarnings("checkstyle:parameterName") + public boolean addAll(final int index, final Collection c) { + throw uoe(); + } + + @Override + @SuppressWarnings("checkstyle:parameterName") + public boolean removeAll(final Collection c) { + throw uoe(); + } + + @Override + @SuppressWarnings("checkstyle:parameterName") + public boolean retainAll(final Collection c) { + throw uoe(); + } + + @Override + @SuppressWarnings("checkstyle:parameterName") + public void sort(final Comparator c) { + throw uoe(); + } + + @Override + public void replaceAll(final UnaryOperator operator) { + throw uoe(); + } + + @Override + protected void removeRange(final int fromIndex, final int toIndex) { + throw uoe(); + } + + private static UnsupportedOperationException uoe() { + return new UnsupportedOperationException("Modification not supported"); + } +} diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/ListNodeCodecContext.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/ListNodeCodecContext.java index 2577b615f1..1774934c43 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/ListNodeCodecContext.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/ListNodeCodecContext.java @@ -7,10 +7,7 @@ */ package org.opendaylight.mdsal.binding.dom.codec.impl; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableList.Builder; import java.lang.reflect.Method; -import java.util.Collection; import java.util.List; import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.yangtools.yang.binding.DataObject; @@ -34,9 +31,9 @@ class ListNodeCodecContext extends DataObjectCodecContext< @Override public D deserialize(final NormalizedNode node) { if (node instanceof MapEntryNode) { - return fromMapEntry((MapEntryNode) node); + return createBindingProxy((MapEntryNode) node); } else if (node instanceof UnkeyedListEntryNode) { - return fromUnkeyedListEntry((UnkeyedListEntryNode) node); + return createBindingProxy((UnkeyedListEntryNode) node); } else { throw new IllegalStateException("Unsupported data type " + node.getClass()); } @@ -47,51 +44,29 @@ class ListNodeCodecContext extends DataObjectCodecContext< if (node instanceof MapNode) { return fromMap((MapNode) node); } else if (node instanceof MapEntryNode) { - return fromMapEntry((MapEntryNode) node); + return createBindingProxy((MapEntryNode) node); } else if (node instanceof UnkeyedListNode) { return fromUnkeyedList((UnkeyedListNode) node); } else if (node instanceof UnkeyedListEntryNode) { - return fromUnkeyedListEntry((UnkeyedListEntryNode) node); + return createBindingProxy((UnkeyedListEntryNode) node); } else { throw new IllegalStateException("Unsupported data type " + node.getClass()); } } + @NonNull Object fromMap(final MapNode map, final int size) { + return LazyBindingList.create(this, size, map.getValue()); + } + private Object fromMap(final MapNode map) { final int size; // This should never happen, but we do need to ensure users never see an empty Map return (size = map.size()) == 0 ? null : fromMap(map, size); } - @NonNull Object fromMap(final MapNode map, final int size) { - // FIXME: Make this a lazily-populated list - final Builder builder = ImmutableList.builderWithExpectedSize(size); - for (MapEntryNode node : map.getValue()) { - builder.add(createBindingProxy(node)); - } - return builder.build(); - } - - final @NonNull D fromMapEntry(final MapEntryNode node) { - return createBindingProxy(node); - } - - private @NonNull D fromUnkeyedListEntry(final UnkeyedListEntryNode node) { - return createBindingProxy(node); - } - - private List fromUnkeyedList(final UnkeyedListNode nodes) { - final Collection value = nodes.getValue(); - if (value.isEmpty()) { - // This should never happen, but we do need to ensure users never see an empty List - return null; - } - - // FIXME: Could be this lazy transformed list? - final Builder builder = ImmutableList.builderWithExpectedSize(value.size()); - for (UnkeyedListEntryNode node : value) { - builder.add(fromUnkeyedListEntry(node)); - } - return builder.build(); + private List fromUnkeyedList(final UnkeyedListNode node) { + final int size; + // This should never happen, but we do need to ensure users never see an empty List + return (size = node.getSize()) == 0 ? null : LazyBindingList.create(this, size, node.getValue()); } } diff --git a/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/AbstractBindingCodecTest.java b/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/AbstractBindingCodecTest.java index 425b440c1d..f49bbf198b 100644 --- a/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/AbstractBindingCodecTest.java +++ b/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/AbstractBindingCodecTest.java @@ -7,9 +7,16 @@ */ package org.opendaylight.mdsal.binding.dom.codec.impl; +import static org.junit.Assert.assertEquals; + +import java.util.Map.Entry; import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; +import org.opendaylight.yangtools.yang.binding.DataObject; +import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; +import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; +import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; public abstract class AbstractBindingCodecTest extends AbstractBindingRuntimeTest { protected BindingCodecContext codecContext; @@ -28,4 +35,13 @@ public abstract class AbstractBindingCodecTest extends AbstractBindingRuntimeTes public void before() { this.codecContext = new BindingCodecContext(getRuntimeContext()); } + + @SuppressWarnings("unchecked") + protected T thereAndBackAgain(final InstanceIdentifier path, final T data) { + final Entry> there = codecContext.toNormalizedNode(path, data); + final Entry, DataObject> backAgain = codecContext.fromNormalizedNode(there.getKey(), + there.getValue()); + assertEquals(path, backAgain.getKey()); + return (T) backAgain.getValue(); + } } diff --git a/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/LazyBindingListTest.java b/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/LazyBindingListTest.java new file mode 100644 index 0000000000..fbcf16818a --- /dev/null +++ b/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/LazyBindingListTest.java @@ -0,0 +1,76 @@ +/* + * Copyright (c) 2020 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.mdsal.binding.dom.codec.impl; + +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThrows; + +import java.util.ArrayList; +import java.util.List; +import org.junit.Test; +import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.mdsal.test.binding.rev140701.Top; +import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.mdsal.test.binding.rev140701.two.level.list.TopLevelList; +import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.mdsal.test.binding.rev140701.two.level.list.TopLevelListBuilder; +import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.mdsal.test.binding.rev140701.two.level.list.top.level.list.NestedList; +import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.mdsal.test.binding.rev140701.two.level.list.top.level.list.NestedListBuilder; +import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; + +public class LazyBindingListTest extends AbstractBindingCodecTest { + @Test + public void testLazyList() { + final List nested = new ArrayList<>(); + for (int i = 0; i < 2 * LazyBindingList.LAZY_CUTOFF; ++i) { + nested.add(new NestedListBuilder().setName(String.valueOf(i)).build()); + } + final TopLevelList expected = new TopLevelListBuilder() + .setName("test") + .setNestedList(nested) + .build(); + final TopLevelList actual = thereAndBackAgain( + InstanceIdentifier.create(Top.class).child(TopLevelList.class, expected.key()), expected); + + final List list = actual.getNestedList(); + assertThat(list, instanceOf(LazyBindingList.class)); + + // Equality does all the right things to check happy paths + assertEquals(expected.getNestedList(), list); + assertEquals(expected.getNestedList().hashCode(), list.hashCode()); + + // Make sure the list performs proper caching + assertSame(list.get(LazyBindingList.LAZY_CUTOFF), list.get(LazyBindingList.LAZY_CUTOFF)); + + // Test throws, just for completeness' sake + assertThrows(UnsupportedOperationException.class, () -> list.add(null)); + assertThrows(UnsupportedOperationException.class, () -> list.addAll(null)); + assertThrows(UnsupportedOperationException.class, () -> list.addAll(0, null)); + assertThrows(UnsupportedOperationException.class, () -> list.remove(null)); + assertThrows(UnsupportedOperationException.class, () -> list.removeAll(null)); + assertThrows(UnsupportedOperationException.class, () -> list.replaceAll(null)); + assertThrows(UnsupportedOperationException.class, () -> list.retainAll(null)); + assertThrows(UnsupportedOperationException.class, () -> list.sort(null)); + assertThrows(UnsupportedOperationException.class, () -> list.clear()); + } + + @Test + public void testSingletonList() { + final TopLevelList expected = new TopLevelListBuilder() + .setName("test") + .setNestedList(List.of(new NestedListBuilder().setName(String.valueOf("one")).build())) + .build(); + final TopLevelList actual = thereAndBackAgain( + InstanceIdentifier.create(Top.class).child(TopLevelList.class, expected.key()), expected); + + final List list = actual.getNestedList(); + assertThat(list, not(instanceOf(LazyBindingList.class))); + assertEquals(expected.getNestedList(), list); + } +}