From a872c7d8cd93c104430f8065c1aa0b69d03e7f3e Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Thu, 28 Oct 2021 13:41:05 +0200 Subject: [PATCH] Compute YangInstanceIdentifier.hashCode() lazily There are very few reasons to eagerly compute the hash code, let's not do that. JIRA: YANGTOOLS-1425 Change-Id: I0e76fa1cac539e318ee7a9bdcd28e6a6a3cbbb68 Signed-off-by: Robert Varga --- .../data/api/FixedYangInstanceIdentifier.java | 35 ++++------ .../yang/data/api/PathArgumentList.java | 1 + .../api/StackedYangInstanceIdentifier.java | 15 ++-- .../yang/data/api/YangInstanceIdentifier.java | 70 ++++++++----------- .../api/YangInstanceIdentifierBuilder.java | 14 ++-- 5 files changed, 57 insertions(+), 78 deletions(-) diff --git a/data/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/FixedYangInstanceIdentifier.java b/data/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/FixedYangInstanceIdentifier.java index 3d4fd97755..4a10de47a3 100644 --- a/data/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/FixedYangInstanceIdentifier.java +++ b/data/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/FixedYangInstanceIdentifier.java @@ -19,19 +19,22 @@ import org.opendaylight.yangtools.util.HashCodeBuilder; final class FixedYangInstanceIdentifier extends YangInstanceIdentifier implements Cloneable { static final @NonNull FixedYangInstanceIdentifier EMPTY_INSTANCE = new FixedYangInstanceIdentifier( - ImmutableList.of(), new HashCodeBuilder<>().build()); + ImmutableList.of()); private static final long serialVersionUID = 1L; private final ImmutableList path; private transient volatile YangInstanceIdentifier parent; - FixedYangInstanceIdentifier(final ImmutableList path, final int hash) { - super(hash); + FixedYangInstanceIdentifier(final ImmutableList path) { this.path = requireNonNull(path, "path must not be null."); } - static @NonNull FixedYangInstanceIdentifier create(final Iterable path, final int hash) { - return new FixedYangInstanceIdentifier(ImmutableList.copyOf(path), hash); + static @NonNull FixedYangInstanceIdentifier of(final ImmutableList path) { + return path.isEmpty() ? EMPTY_INSTANCE : new FixedYangInstanceIdentifier(path); + } + + static @NonNull FixedYangInstanceIdentifier of(final List path) { + return path.isEmpty() ? EMPTY_INSTANCE : new FixedYangInstanceIdentifier(ImmutableList.copyOf(path)); } @Override @@ -110,17 +113,8 @@ final class FixedYangInstanceIdentifier extends YangInstanceIdentifier implement @Override YangInstanceIdentifier createRelativeIdentifier(final int skipFromRoot) { - if (skipFromRoot == path.size()) { - return EMPTY_INSTANCE; - } - - final ImmutableList newPath = path.subList(skipFromRoot, path.size()); - final HashCodeBuilder hash = new HashCodeBuilder<>(); - for (PathArgument a : newPath) { - hash.addArgument(a); - } - - return new FixedYangInstanceIdentifier(newPath, hash.build()); + return skipFromRoot == path.size() ? EMPTY_INSTANCE + : new FixedYangInstanceIdentifier(path.subList(skipFromRoot, path.size())); } private Object readResolve() throws ObjectStreamException { @@ -128,11 +122,12 @@ final class FixedYangInstanceIdentifier extends YangInstanceIdentifier implement } @Override - boolean pathArgumentsEqual(final YangInstanceIdentifier other) { - if (other instanceof FixedYangInstanceIdentifier) { - return path.equals(((FixedYangInstanceIdentifier) other).path); + int computeHashCode() { + int ret = 1; + for (PathArgument arg : path) { + ret = HashCodeBuilder.nextHashCode(ret, arg); } - return super.pathArgumentsEqual(other); + return ret; } @Override diff --git a/data/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/PathArgumentList.java b/data/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/PathArgumentList.java index e41d79b711..e0ff756808 100644 --- a/data/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/PathArgumentList.java +++ b/data/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/PathArgumentList.java @@ -13,6 +13,7 @@ import java.util.Collection; import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument; +// FIXME: sealed once we have JDK17+ abstract class PathArgumentList extends AbstractList { @Override public abstract @NonNull UnmodifiableIterator iterator(); diff --git a/data/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/StackedYangInstanceIdentifier.java b/data/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/StackedYangInstanceIdentifier.java index c8ff903394..b2cc4497da 100644 --- a/data/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/StackedYangInstanceIdentifier.java +++ b/data/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/StackedYangInstanceIdentifier.java @@ -23,6 +23,7 @@ import java.security.PrivilegedAction; import java.util.ArrayList; import java.util.List; import org.eclipse.jdt.annotation.NonNull; +import org.opendaylight.yangtools.util.HashCodeBuilder; final class StackedYangInstanceIdentifier extends YangInstanceIdentifier implements Cloneable { private static final long serialVersionUID = 1L; @@ -50,9 +51,7 @@ final class StackedYangInstanceIdentifier extends YangInstanceIdentifier impleme private transient volatile StackedPathArguments pathArguments; private transient volatile StackedReversePathArguments reversePathArguments; - StackedYangInstanceIdentifier(final YangInstanceIdentifier parent, final PathArgument pathArgument, - final int hash) { - super(hash); + StackedYangInstanceIdentifier(final YangInstanceIdentifier parent, final PathArgument pathArgument) { this.parent = requireNonNull(parent); this.pathArgument = requireNonNull(pathArgument); } @@ -126,8 +125,7 @@ final class StackedYangInstanceIdentifier extends YangInstanceIdentifier impleme current = stacked.getParent(); } while (current.tryPathArguments() == null); - ret = new StackedPathArguments(current, Lists.reverse(stack)); - pathArguments = ret; + pathArguments = ret = new StackedPathArguments(current, Lists.reverse(stack)); } return ret; @@ -164,6 +162,11 @@ final class StackedYangInstanceIdentifier extends YangInstanceIdentifier impleme return YangInstanceIdentifier.create(Iterables.skip(getPathArguments(), skipFromRoot)); } + @Override + int computeHashCode() { + return HashCodeBuilder.nextHashCode(parent.hashCode(), pathArgument); + } + @Override boolean pathArgumentsEqual(final YangInstanceIdentifier other) { if (other instanceof StackedYangInstanceIdentifier) { @@ -191,7 +194,7 @@ final class StackedYangInstanceIdentifier extends YangInstanceIdentifier impleme if (parent instanceof FixedYangInstanceIdentifier) { p = (FixedYangInstanceIdentifier) parent; } else { - p = FixedYangInstanceIdentifier.create(parent.getPathArguments(), parent.hashCode()); + p = FixedYangInstanceIdentifier.of(parent.getPathArguments()); } outputStream.writeObject(p); } diff --git a/data/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/YangInstanceIdentifier.java b/data/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/YangInstanceIdentifier.java index 8c2204dd6d..f0d9627fe8 100644 --- a/data/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/YangInstanceIdentifier.java +++ b/data/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/YangInstanceIdentifier.java @@ -43,7 +43,6 @@ import org.eclipse.jdt.annotation.Nullable; import org.opendaylight.yangtools.concepts.HierarchicalIdentifier; import org.opendaylight.yangtools.concepts.Immutable; import org.opendaylight.yangtools.concepts.Mutable; -import org.opendaylight.yangtools.util.HashCodeBuilder; import org.opendaylight.yangtools.util.ImmutableOffsetMap; import org.opendaylight.yangtools.util.SingletonSet; import org.opendaylight.yangtools.yang.common.QName; @@ -76,30 +75,29 @@ import org.opendaylight.yangtools.yang.data.api.schema.LeafSetEntryNode; * * @see RFC6020 */ -// FIXME: 7.0.0: this concept needs to be moved to yang-common, as parser components need the ability to refer -// to data nodes -- most notably XPath expressions and {@code default} statement arguments need to be able -// to represent these. +// FIXME: sealed once we have JDK17+ public abstract class YangInstanceIdentifier implements HierarchicalIdentifier { private static final long serialVersionUID = 4L; private static final VarHandle TO_STRING_CACHE; + private static final VarHandle HASH; static { + final var lookup = MethodHandles.lookup(); try { - TO_STRING_CACHE = MethodHandles.lookup().findVarHandle(YangInstanceIdentifier.class, "toStringCache", - String.class); + HASH = lookup.findVarHandle(YangInstanceIdentifier.class, "hash", int.class); + TO_STRING_CACHE = lookup.findVarHandle(YangInstanceIdentifier.class, "toStringCache", String.class); } catch (NoSuchFieldException | IllegalAccessException e) { throw new ExceptionInInitializerError(e); } } - - private final int hash; + @SuppressWarnings("unused") + private int hash; @SuppressWarnings("unused") private transient String toStringCache = null; - // Package-private to prevent outside subclassing - YangInstanceIdentifier(final int hash) { - this.hash = hash; + YangInstanceIdentifier() { + // Package-private to prevent outside subclassing } /** @@ -114,9 +112,9 @@ public abstract class YangInstanceIdentifier implements HierarchicalIdentifier tryPathArguments(); + abstract @Nullable List tryPathArguments(); - abstract @Nullable Collection tryReversePathArguments(); + abstract @Nullable List tryReversePathArguments(); /** * Check if this instance identifier has empty path arguments, e.g. it is @@ -184,21 +182,11 @@ public abstract class YangInstanceIdentifier implements HierarchicalIdentifier path) { - if (Iterables.isEmpty(path)) { - return empty(); - } - - final HashCodeBuilder hash = new HashCodeBuilder<>(); - for (PathArgument a : path) { - hash.addArgument(a); - } - - return FixedYangInstanceIdentifier.create(path, hash.build()); + return Iterables.isEmpty(path) ? empty() : new FixedYangInstanceIdentifier(ImmutableList.copyOf(path)); } public static @NonNull YangInstanceIdentifier create(final PathArgument pathArgument) { - return new FixedYangInstanceIdentifier(ImmutableList.of(pathArgument), - HashCodeBuilder.nextHashCode(1, pathArgument)); + return new FixedYangInstanceIdentifier(ImmutableList.of(pathArgument)); } public static @NonNull YangInstanceIdentifier create(final PathArgument... path) { @@ -240,23 +228,12 @@ public abstract class YangInstanceIdentifier implements HierarchicalIdentifier hash; private final List path; YangInstanceIdentifierBuilder() { - this.hash = new HashCodeBuilder<>(); - this.path = new ArrayList<>(); + path = new ArrayList<>(); } - YangInstanceIdentifierBuilder(final List prefix, final int hash) { - this.path = new ArrayList<>(prefix); - this.hash = new HashCodeBuilder<>(hash); + YangInstanceIdentifierBuilder(final List prefix) { + path = new ArrayList<>(prefix); } private @NonNull InstanceIdentifierBuilder addArgument(final PathArgument arg) { path.add(arg); - hash.addArgument(arg); return this; } @@ -54,7 +49,6 @@ final class YangInstanceIdentifierBuilder implements InstanceIdentifierBuilder { @Override public InstanceIdentifierBuilder append(final Collection args) { path.addAll(args); - args.forEach(hash::addArgument); return this; } @@ -70,6 +64,6 @@ final class YangInstanceIdentifierBuilder implements InstanceIdentifierBuilder { @Override public YangInstanceIdentifier build() { - return FixedYangInstanceIdentifier.create(path, hash.build()); + return FixedYangInstanceIdentifier.of(path); } } -- 2.36.6