From 3174ac0dc4fb109977fc21f95c82c07e4392172c Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Sun, 17 Jan 2021 10:00:23 +0100 Subject: [PATCH] Separate out StatementSupport.applyCopyPolicy() We have two concerns here: - copying statements for inference purposes and lazily instantiating them there - creating EffectiveStatement copies. StatementSupport.copyPolicy() and StatementContextBase.copyAsChildOf() take care of the first part. StatementSupport.effectiveCopyOf() and StatementContextBase.asEffectiveChildOf() are responsible for the second concern. For now they are not wired, but concentrate remaining FIXMEs for direction we need to in. JIRA: YANGTOOLS-1195 Change-Id: I3b0a63b4620cf933ce104dc37e9db8678c71aba5 Signed-off-by: Robert Varga --- .../reactor/InferredStatementContext.java | 4 ++ .../stmt/reactor/StatementContextBase.java | 45 ++++++++++---- .../spi/meta/AbstractStatementSupport.java | 19 +++++- .../spi/meta/ForwardingStatementSupport.java | 9 ++- .../parser/spi/meta/StatementSupport.java | 61 +++++++++++++------ 5 files changed, 105 insertions(+), 33 deletions(-) diff --git a/yang/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/InferredStatementContext.java b/yang/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/InferredStatementContext.java index 1f062e62e6..fb691e3c31 100644 --- a/yang/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/InferredStatementContext.java +++ b/yang/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/InferredStatementContext.java @@ -376,6 +376,10 @@ final class InferredStatementContext, E extend } private Optional> copySubstatement(final Mutable substatement) { + // FIXME: YANGTOOLS-1195: this is not exactly what we want to do here, because we are deling with two different + // requests: copy for inference purposes (this method), while we also copy for purposes + // of buildEffective() -- in which case we want to probably invoke asEffectiveChildOf() + // or similar return substatement.copyAsChildOf(this, childCopyType, targetModule); } diff --git a/yang/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/StatementContextBase.java b/yang/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/StatementContextBase.java index 899345574a..cd07b95b1c 100644 --- a/yang/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/StatementContextBase.java +++ b/yang/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/StatementContextBase.java @@ -597,23 +597,15 @@ public abstract class StatementContextBase, E checkEffectiveModelCompleted(this); final StatementSupport support = definition.support(); - final CopyPolicy policy = support.applyCopyPolicy(this, parent, type, targetModule); + final CopyPolicy policy = support.copyPolicy(); switch (policy) { case CONTEXT_INDEPENDENT: - if (hasEmptySubstatements()) { - // This statement is context-independent and has no substatements -- hence it can be freely shared. + if (substatementsContextIndependent()) { return Optional.of(replicaAsChildOf(parent)); } - // ascertaining substatements could be quite costly, let's just fall through to declared copy and deal - // shortcut it when we build the statements. + // fall through case DECLARED_COPY: - // FIXME: YANGTOOLS-694: this is still to eager, we really want to copy as a lazily-instantiated - // context, so that we can support building an effective statement without copying - // anything -- we will typically end up not being inferred against. In that case, - // this slim context should end up dealing with differences at buildContext() - // time. This is a YANGTOOLS-1067 prerequisite (which will deal with what can and - // cannot be shared across instances). return Optional.of(parent.childCopyOf(this, type, targetModule)); case IGNORE: return Optional.empty(); @@ -624,6 +616,37 @@ public abstract class StatementContextBase, E } } + private boolean substatementsContextIndependent() { + // FIXME: YANGTOOLS-1195: we really want to compute (and cache) the summary for substatements. + // + // For now we just check if there are any substatements, but we really want to ask: + // + // Are all substatements (recursively) CONTEXT_INDEPENDENT as well? + // + // Which is something we want to compute once and store. This needs to be implemented. + return hasEmptySubstatements(); + } + + // FIXME: YANGTOOLS-1195: this method is unused, but should be called from InferredStatementContext at the very + // least. It should return @NonNull -- either 'E' or EffectiveStmtCtx.Current'. Perhaps its arguments need + // to be adjusted, too. + final void asEffectiveChildOf(final Mutable parent, final CopyType type, final QNameModule targetModule) { + checkEffectiveModelCompleted(this); + + final StatementSupport support = definition.support(); + final StmtContext effective = support.effectiveCopyOf(this, parent, type, targetModule); + if (effective == this) { + LOG.debug("Should reuse {}", this); + return; + } + + // FIXME: YANGTOOLS-1195: here is probably where we want to do some statement reuse: even if the parent is + // affected, some substatements may not -- in which case we want to reuse them. This + // probably needs to be a callout of some kind. + // FIXME: YANGTOOLS-1067: an incremental improvement to that is that if no substatements changed, we want to + // be reusing the entire List and pass that as substatements. + } + @Override public final Mutable childCopyOf(final StmtContext stmt, final CopyType type, final QNameModule targetModule) { diff --git a/yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/AbstractStatementSupport.java b/yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/AbstractStatementSupport.java index 4e11270681..0622463b82 100644 --- a/yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/AbstractStatementSupport.java +++ b/yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/AbstractStatementSupport.java @@ -11,6 +11,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static java.util.Objects.requireNonNull; import com.google.common.annotations.Beta; +import com.google.common.base.VerifyException; import org.eclipse.jdt.annotation.NonNull; import org.eclipse.jdt.annotation.Nullable; import org.opendaylight.yangtools.yang.common.QNameModule; @@ -46,11 +47,25 @@ public abstract class AbstractStatementSupport } @Override - public CopyPolicy applyCopyPolicy(final Mutable stmt, final Mutable parent, - final CopyType copyType, final QNameModule targetModule) { + public final CopyPolicy copyPolicy() { return copyPolicy; } + @Override + public final StmtContext effectiveCopyOf(final StmtContext stmt, final Mutable parent, + final CopyType copyType, final QNameModule targetModule) { + switch (copyPolicy) { + case CONTEXT_INDEPENDENT: + return stmt; + case DECLARED_COPY: + // FIXME: YANGTOOLS-1195: this is too harsh, we need to make a callout to subclass methods so they + // actually examine the differences. + return parent.childCopyOf(stmt, copyType, targetModule); + default: + throw new VerifyException("Attempted to apply " + copyPolicy); + } + } + @Override public void onStatementAdded(final StmtContext.Mutable stmt) { // NOOP for most implementations diff --git a/yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/ForwardingStatementSupport.java b/yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/ForwardingStatementSupport.java index 1e8eca6b2c..827a97bdc9 100644 --- a/yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/ForwardingStatementSupport.java +++ b/yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/ForwardingStatementSupport.java @@ -92,8 +92,13 @@ public abstract class ForwardingStatementSupport stmt, final Mutable parent, + public CopyPolicy copyPolicy() { + return delegate().copyPolicy(); + } + + @Override + public StmtContext effectiveCopyOf(final StmtContext stmt, final Mutable parent, final CopyType copyType, final QNameModule targetModule) { - return delegate().applyCopyPolicy(stmt, parent, copyType, targetModule); + return delegate().effectiveCopyOf(stmt, parent, copyType, targetModule); } } diff --git a/yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/StatementSupport.java b/yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/StatementSupport.java index a1be785a6b..419c28c60d 100644 --- a/yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/StatementSupport.java +++ b/yang/yang-parser-spi/src/main/java/org/opendaylight/yangtools/yang/parser/spi/meta/StatementSupport.java @@ -141,7 +141,42 @@ public interface StatementSupport, E extends E @Nullable StatementSupport getSupportSpecificForArgument(String argument); /** - * Determine reactor copy behavior of a statement instance. Statement support classes are required to determine + * Return this statement's {@link CopyPolicy}. This is a static value, reflecting how this statement reacts to being + * replicated to a different context, without reflecting on behaviour of potential substatements, which would come + * into play in something like: + * + * 
+     *   
+     *     module foo {
+     *       namespace foo;
+     *       prefix foo;
+     *
+     *       extension note {
+     *         argument string {
+     *           type string {
+     *             length 1..max;
+     *           }
+     *         }
+     *         description "Can be used in description/reference statements to attach additional notes";
+     *       }
+     *
+     *       description "A nice module extending description statement semantics" {
+     *         foo:note "We can now attach description/reference a note.";
+     *         foo:note "Also another note";
+     *       }
+     *     }
+     *   
+     *
+ * + *

+ * In this scenario, it is the reactor's job to figure out what to do (like talking to substatements). + * + * @return This statement's copy policy + */ + @NonNull CopyPolicy copyPolicy(); + + /** + * Determine reactor copy behaviour of a statement instance. Statement support classes are required to determine * their operations with regard to their statements being replicated into different contexts, so that * {@link Mutable} instances are not created when it is evident they are superfluous. * @@ -156,26 +191,17 @@ public interface StatementSupport, E extends E * the default namespace to parent's namespace, whereas {@code augment} does not. * * + *

+ * Implementations should return the context to use -- returning {@code stmt} if there is no change or a copy of it. + * * @param stmt Context of statement to be copied statement * @param parent Parent statement context * @param copyType Type of copy being performed * @param targetModule Target module, if present - * @return Policy that needs to be applied to the copy operation of this statement. + * @return StmtContext holding the effective state */ - default @NonNull CopyPolicy applyCopyPolicy(final Mutable stmt, final Mutable parent, - final CopyType copyType, @Nullable final QNameModule targetModule) { - // Most of statement supports will just want to copy the statement - // FIXME: YANGTOOLS-694: that is not strictly true. Subclasses of this should indicate if they are themselves - // copy-sensitive: - // 1) if they are not and cannot be targeted by inference, and all their current - // substatements are also non-sensitive, we want to return the same context. - // 2) if they are not and their current substatements are sensitive, we want to copy - // as a lazily-instantiated interceptor to let it deal with substatements when needed - // (YANGTOOLS-1067 prerequisite) - // 3) otherwise perform this eager copy - // return Optional.of(parent.childCopyOf(stmt, copyType, targetModule)); - return CopyPolicy.DECLARED_COPY; - } + @NonNull StmtContext effectiveCopyOf(StmtContext stmt, Mutable parent, CopyType copyType, + @Nullable QNameModule targetModule); /** * Given a raw string representation of an argument, try to use a shared representation. @@ -247,8 +273,7 @@ public interface StatementSupport, E extends E /** * Statement context copy policy, indicating how should reactor handle statement copy operations. Every statement * copied by the reactor is subject to policy check done by - * {@link StatementSupport#applyCopyPolicy(Mutable, Mutable, CopyType, QNameModule)}. - * + * {@link StatementSupport#effectiveCopyOf(StmtContext, Mutable, CopyType, QNameModule)}. */ enum CopyPolicy { /** -- 2.36.6