From: Robert Varga Date: Fri, 3 Jul 2020 14:52:15 +0000 (+0200) Subject: Refactor ExtensionStatementSupport X-Git-Tag: v5.0.4~10 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=8ae45ea2e2a4d08f70799a6cd8984698427bf6e4;p=yangtools.git Refactor ExtensionStatementSupport Using RecursiveObjectLeaker requires integration with object construction and is inherently dangerous. As it turns out, we can solve this problem differently, without having to rely on this magic by pre-allocating the resulting effective statement and populating it into a thread-local map. That allows us to pick up that object for purposes of including it in substatements -- thus breaking the recursion. Once we have acquired substatements, the real build methods just fill them into the pre-allocated object are return it. On exit we check whether we have cleared the state map and clean it up automatically, as this is not expected to be a major performance problem. JIRA: YANGTOOLS-1122 Change-Id: Iebbbffd6f62fa57ce496a2ab7bc8e5792198d3a5 Signed-off-by: Robert Varga --- diff --git a/yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/stmt/ExtensionEffectiveStatement.java b/yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/stmt/ExtensionEffectiveStatement.java index d8a8586c9a..346c332d49 100644 --- a/yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/stmt/ExtensionEffectiveStatement.java +++ b/yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/stmt/ExtensionEffectiveStatement.java @@ -8,8 +8,13 @@ package org.opendaylight.yangtools.yang.model.api.stmt; import com.google.common.annotations.Beta; +import org.opendaylight.yangtools.yang.model.api.YangStmtMapping; +import org.opendaylight.yangtools.yang.model.api.meta.StatementDefinition; @Beta public interface ExtensionEffectiveStatement extends NamespacedEffectiveStatement { - + @Override + default StatementDefinition statementDefinition() { + return YangStmtMapping.EXTENSION; + } } diff --git a/yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/stmt/ExtensionStatement.java b/yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/stmt/ExtensionStatement.java index 01f712047d..b97835cbed 100644 --- a/yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/stmt/ExtensionStatement.java +++ b/yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/stmt/ExtensionStatement.java @@ -10,8 +10,15 @@ package org.opendaylight.yangtools.yang.model.api.stmt; import java.util.Optional; import org.eclipse.jdt.annotation.Nullable; import org.opendaylight.yangtools.yang.common.QName; +import org.opendaylight.yangtools.yang.model.api.YangStmtMapping; +import org.opendaylight.yangtools.yang.model.api.meta.StatementDefinition; public interface ExtensionStatement extends DocumentedDeclaredStatement.WithStatus { + @Override + default StatementDefinition statementDefinition() { + return YangStmtMapping.EXTENSION; + } + default @Nullable ArgumentStatement getArgument() { final Optional opt = findFirstDeclaredSubstatement(ArgumentStatement.class); return opt.isPresent() ? opt.get() : null; diff --git a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/BaseQNameStatementSupport.java b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/BaseQNameStatementSupport.java index bbc2d6c94a..5af1ba2783 100644 --- a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/BaseQNameStatementSupport.java +++ b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/BaseQNameStatementSupport.java @@ -48,7 +48,7 @@ public abstract class BaseQNameStatementSupport ctx); @Override - public final E createEffective(final StmtContext ctx) { + public E createEffective(final StmtContext ctx) { final D declared = ctx.buildDeclared(); final ImmutableList> substatements = BaseStatementSupport.buildEffectiveSubstatements(ctx); diff --git a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/extension/ExtensionStatementImpl.java b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/extension/EmptyExtensionStatement.java similarity index 58% rename from yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/extension/ExtensionStatementImpl.java rename to yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/extension/EmptyExtensionStatement.java index 97e663d451..930e51515e 100644 --- a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/extension/ExtensionStatementImpl.java +++ b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/extension/EmptyExtensionStatement.java @@ -9,11 +9,10 @@ package org.opendaylight.yangtools.yang.parser.rfc7950.stmt.extension; import org.opendaylight.yangtools.yang.common.QName; import org.opendaylight.yangtools.yang.model.api.stmt.ExtensionStatement; -import org.opendaylight.yangtools.yang.parser.spi.meta.AbstractDeclaredStatement; -import org.opendaylight.yangtools.yang.parser.spi.meta.StmtContext; +import org.opendaylight.yangtools.yang.parser.rfc7950.stmt.AbstractDeclaredStatement.WithQNameArgument; -final class ExtensionStatementImpl extends AbstractDeclaredStatement implements ExtensionStatement { - ExtensionStatementImpl(final StmtContext context) { - super(context); +final class EmptyExtensionStatement extends WithQNameArgument implements ExtensionStatement { + EmptyExtensionStatement(final QName argument) { + super(argument); } } diff --git a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/extension/ExtensionEffectiveStatementImpl.java b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/extension/ExtensionEffectiveStatementImpl.java index e3de5c550b..13ef829a59 100644 --- a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/extension/ExtensionEffectiveStatementImpl.java +++ b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/extension/ExtensionEffectiveStatementImpl.java @@ -7,26 +7,29 @@ */ package org.opendaylight.yangtools.yang.parser.rfc7950.stmt.extension; +import static com.google.common.base.Verify.verify; +import static com.google.common.base.Verify.verifyNotNull; +import static java.util.Objects.requireNonNull; + +import com.google.common.collect.ImmutableList; import java.util.ArrayDeque; -import java.util.Collection; import java.util.Deque; -import java.util.Optional; import org.eclipse.jdt.annotation.NonNull; -import org.eclipse.jdt.annotation.Nullable; -import org.opendaylight.yangtools.util.RecursiveObjectLeaker; import org.opendaylight.yangtools.yang.common.QName; import org.opendaylight.yangtools.yang.model.api.ExtensionDefinition; import org.opendaylight.yangtools.yang.model.api.SchemaPath; +import org.opendaylight.yangtools.yang.model.api.Status; import org.opendaylight.yangtools.yang.model.api.meta.EffectiveStatement; import org.opendaylight.yangtools.yang.model.api.stmt.ArgumentEffectiveStatement; import org.opendaylight.yangtools.yang.model.api.stmt.ExtensionEffectiveStatement; import org.opendaylight.yangtools.yang.model.api.stmt.ExtensionStatement; +import org.opendaylight.yangtools.yang.model.api.stmt.StatusEffectiveStatement; import org.opendaylight.yangtools.yang.model.api.stmt.YinElementEffectiveStatement; -import org.opendaylight.yangtools.yang.parser.rfc7950.stmt.AbstractEffectiveDocumentedNodeWithStatus; -import org.opendaylight.yangtools.yang.parser.spi.meta.StmtContext; +import org.opendaylight.yangtools.yang.parser.rfc7950.stmt.AbstractDeclaredEffectiveStatement.DefaultArgument; +import org.opendaylight.yangtools.yang.parser.rfc7950.stmt.EffectiveStatementMixins.DocumentedNodeMixin; -final class ExtensionEffectiveStatementImpl extends AbstractEffectiveDocumentedNodeWithStatus - implements ExtensionDefinition, ExtensionEffectiveStatement { +final class ExtensionEffectiveStatementImpl extends DefaultArgument + implements ExtensionDefinition, ExtensionEffectiveStatement, DocumentedNodeMixin { private static final class RecursionDetector extends ThreadLocal> { boolean check(final ExtensionEffectiveStatementImpl current) { final Deque stack = get(); @@ -61,85 +64,50 @@ final class ExtensionEffectiveStatementImpl extends AbstractEffectiveDocumentedN private static final RecursionDetector TOSTRING_DETECTOR = new RecursionDetector(); - private final @NonNull QName qname; - private final @Nullable String argument; - private final @NonNull SchemaPath schemaPath; - - private final boolean yin; - - private ExtensionEffectiveStatementImpl( - final StmtContext ctx) { - super(ctx); - this.qname = ctx.coerceStatementArgument(); - this.schemaPath = ctx.getSchemaPath().get(); - - // initFields - final Optional optArgumentSubstatement = findFirstEffectiveSubstatement( - ArgumentEffectiveStatement.class); - if (optArgumentSubstatement.isPresent()) { - final ArgumentEffectiveStatement argumentStatement = optArgumentSubstatement.get(); - this.argument = argumentStatement.argument().getLocalName(); - this.yin = argumentStatement.findFirstEffectiveSubstatement(YinElementEffectiveStatement.class) - .map(YinElementEffectiveStatement::argument).orElse(Boolean.FALSE).booleanValue(); - } else { - this.argument = null; - this.yin = false; - } - } + private final @NonNull SchemaPath path; - /** - * Create a new ExtensionEffectiveStatement, dealing with potential recursion. - * - * @param ctx Statement context - * @return A potentially under-initialized instance - */ - static ExtensionEffectiveStatement create( - final StmtContext ctx) { - // Look at the thread-local leak in case we are invoked recursively - final ExtensionEffectiveStatementImpl existing = RecursiveObjectLeaker.lookup(ctx, - ExtensionEffectiveStatementImpl.class); - if (existing != null) { - // Careful! this object is not fully initialized! - return existing; - } + private volatile Object substatements; - RecursiveObjectLeaker.beforeConstructor(ctx); - try { - // This result is fine, we know it has been completely initialized - return new ExtensionEffectiveStatementImpl(ctx); - } finally { - RecursiveObjectLeaker.afterConstructor(ctx); - } - } - - @Override - protected Collection> initSubstatements( - final Collection> substatementsInit) { - // WARNING: this leaks an incompletely-initialized object - RecursiveObjectLeaker.inConstructor(this); - - return super.initSubstatements(substatementsInit); + ExtensionEffectiveStatementImpl(final ExtensionStatement declared, final SchemaPath path) { + super(declared); + this.path = requireNonNull(path); } @Override public QName getQName() { - return qname; + return argument(); } @Override @Deprecated public SchemaPath getPath() { - return schemaPath; + return path; } @Override public String getArgument() { - return argument; + return findFirstEffectiveSubstatementArgument(ArgumentEffectiveStatement.class) + .map(QName::getLocalName) + .orElse(null); } @Override public boolean isYinElement() { - return yin; + return findFirstEffectiveSubstatement(ArgumentEffectiveStatement.class) + .flatMap(arg -> arg.findFirstEffectiveSubstatementArgument(YinElementEffectiveStatement.class)) + .orElse(Boolean.FALSE) + .booleanValue(); + } + + @Override + public ImmutableList> effectiveSubstatements() { + final Object local = verifyNotNull(substatements, "Substatements are not yet initialized"); + return unmaskList(local); + } + + @Override + public Status getStatus() { + return findFirstEffectiveSubstatementArgument(StatusEffectiveStatement.class).orElse(Status.CURRENT); } @Override @@ -151,23 +119,28 @@ final class ExtensionEffectiveStatementImpl extends AbstractEffectiveDocumentedN TOSTRING_DETECTOR.push(this); try { return ExtensionEffectiveStatementImpl.class.getSimpleName() + "[" - + "argument=" + argument - + ", qname=" + qname - + ", schemaPath=" + schemaPath + + "argument=" + getArgument() + + ", qname=" + getQName() + + ", schemaPath=" + path + + ", yin=" + isYinElement() + ", extensionSchemaNodes=" + getUnknownSchemaNodes() - + ", yin=" + yin + "]"; } finally { TOSTRING_DETECTOR.pop(); } } + void setSubstatements(final ImmutableList> newSubstatements) { + verify(substatements == null, "Substatements already initialized"); + substatements = maskList(newSubstatements); + } + private String recursedToString() { return ExtensionEffectiveStatementImpl.class.getSimpleName() + "[" - + "argument=" + argument - + ", qname=" + qname - + ", schemaPath=" + schemaPath - + ", yin=" + yin + + "argument=" + getArgument() + + ", qname=" + getQName() + + ", schemaPath=" + path + + ", yin=" + isYinElement() + " ]"; } } diff --git a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/extension/ExtensionStatementSupport.java b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/extension/ExtensionStatementSupport.java index 144b7f1cdd..1752e70982 100644 --- a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/extension/ExtensionStatementSupport.java +++ b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/extension/ExtensionStatementSupport.java @@ -7,15 +7,24 @@ */ package org.opendaylight.yangtools.yang.parser.rfc7950.stmt.extension; +import static com.google.common.base.Verify.verify; +import static com.google.common.base.Verify.verifyNotNull; + +import com.google.common.collect.ImmutableList; +import java.util.IdentityHashMap; +import java.util.Map; +import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.yangtools.openconfig.model.api.OpenConfigStatements; import org.opendaylight.yangtools.yang.common.QName; import org.opendaylight.yangtools.yang.model.api.YangStmtMapping; +import org.opendaylight.yangtools.yang.model.api.meta.DeclaredStatement; +import org.opendaylight.yangtools.yang.model.api.meta.EffectiveStatement; import org.opendaylight.yangtools.yang.model.api.stmt.ArgumentStatement; import org.opendaylight.yangtools.yang.model.api.stmt.ExtensionEffectiveStatement; import org.opendaylight.yangtools.yang.model.api.stmt.ExtensionStatement; import org.opendaylight.yangtools.yang.model.api.stmt.YinElementStatement; +import org.opendaylight.yangtools.yang.parser.rfc7950.stmt.BaseQNameStatementSupport; import org.opendaylight.yangtools.yang.parser.spi.ExtensionNamespace; -import org.opendaylight.yangtools.yang.parser.spi.meta.AbstractQNameStatementSupport; import org.opendaylight.yangtools.yang.parser.spi.meta.StatementDefinitionNamespace; import org.opendaylight.yangtools.yang.parser.spi.meta.StmtContext; import org.opendaylight.yangtools.yang.parser.spi.meta.StmtContext.Mutable; @@ -23,7 +32,7 @@ import org.opendaylight.yangtools.yang.parser.spi.meta.StmtContextUtils; import org.opendaylight.yangtools.yang.parser.spi.meta.SubstatementValidator; public final class ExtensionStatementSupport - extends AbstractQNameStatementSupport { + extends BaseQNameStatementSupport { private static final SubstatementValidator SUBSTATEMENT_VALIDATOR = SubstatementValidator.builder(YangStmtMapping .EXTENSION) .addOptional(YangStmtMapping.ARGUMENT) @@ -32,6 +41,8 @@ public final class ExtensionStatementSupport .addOptional(YangStmtMapping.STATUS) .build(); private static final ExtensionStatementSupport INSTANCE = new ExtensionStatementSupport(); + private static final ThreadLocal, ExtensionEffectiveStatementImpl>> TL_BUILDERS = + new ThreadLocal<>(); private ExtensionStatementSupport() { super(YangStmtMapping.EXTENSION); @@ -46,17 +57,6 @@ public final class ExtensionStatementSupport return StmtContextUtils.parseIdentifier(ctx, value); } - @Override - public ExtensionStatement createDeclared(final StmtContext ctx) { - return new ExtensionStatementImpl(ctx); - } - - @Override - public ExtensionEffectiveStatement createEffective( - final StmtContext ctx) { - return ExtensionEffectiveStatementImpl.create(ctx); - } - @Override public void onStatementDefinitionDeclared( final Mutable stmt) { @@ -84,4 +84,70 @@ public final class ExtensionStatementSupport protected SubstatementValidator getSubstatementValidator() { return SUBSTATEMENT_VALIDATOR; } + + @Override + protected ExtensionStatement createDeclared(final StmtContext ctx, + final ImmutableList> substatements) { + return new RegularExtensionStatement(ctx.coerceStatementArgument(), substatements); + } + + @Override + protected ExtensionStatement createEmptyDeclared(final StmtContext ctx) { + return new EmptyExtensionStatement(ctx.coerceStatementArgument()); + } + + @Override + public ExtensionEffectiveStatement createEffective( + final StmtContext ctx) { + Map, ExtensionEffectiveStatementImpl> tl = TL_BUILDERS.get(); + if (tl == null) { + tl = new IdentityHashMap<>(); + TL_BUILDERS.set(tl); + } + + final ExtensionEffectiveStatementImpl existing = tl.get(ctx); + if (existing != null) { + // Implies non-empty map, no cleanup necessary + return existing; + } + + try { + final ExtensionEffectiveStatementImpl created = new ExtensionEffectiveStatementImpl(ctx.buildDeclared(), + ctx.getSchemaPath().get()); + verify(tl.put(ctx, created) == null); + try { + return super.createEffective(ctx); + } finally { + verify(tl.remove(ctx) == created); + + } + } finally { + if (tl.isEmpty()) { + TL_BUILDERS.remove(); + } + } + } + + @Override + protected ExtensionEffectiveStatement createEffective( + final StmtContext ctx, + final ExtensionStatement declared, + final ImmutableList> substatements) { + return finishCreate(ctx, substatements); + } + + @Override + protected ExtensionEffectiveStatement createEmptyEffective( + final StmtContext ctx, + final ExtensionStatement declared) { + return finishCreate(ctx, ImmutableList.of()); + } + + private static @NonNull ExtensionEffectiveStatement finishCreate(final StmtContext ctx, + final ImmutableList> substatements) { + final ExtensionEffectiveStatementImpl ret = verifyNotNull(verifyNotNull(TL_BUILDERS.get(), + "Statement build state not initialized").get(ctx), "No build state found for %s", ctx); + ret.setSubstatements(substatements); + return ret; + } } \ No newline at end of file diff --git a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/extension/RegularExtensionStatement.java b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/extension/RegularExtensionStatement.java new file mode 100644 index 0000000000..7baacd9fd2 --- /dev/null +++ b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/extension/RegularExtensionStatement.java @@ -0,0 +1,20 @@ +/* + * 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.yangtools.yang.parser.rfc7950.stmt.extension; + +import com.google.common.collect.ImmutableList; +import org.opendaylight.yangtools.yang.common.QName; +import org.opendaylight.yangtools.yang.model.api.meta.DeclaredStatement; +import org.opendaylight.yangtools.yang.model.api.stmt.ExtensionStatement; +import org.opendaylight.yangtools.yang.parser.rfc7950.stmt.AbstractDeclaredStatement.WithQNameArgument.WithSubstatements; + +final class RegularExtensionStatement extends WithSubstatements implements ExtensionStatement { + RegularExtensionStatement(final QName argument, final ImmutableList> substatements) { + super(argument, substatements); + } +}