Sweep child->parent relationships more quickly 98/93998/7
authorRobert Varga <robert.varga@pantheon.tech>
Tue, 1 Dec 2020 22:33:10 +0000 (23:33 +0100)
committerRobert Varga <robert.varga@pantheon.tech>
Wed, 2 Dec 2020 10:39:30 +0000 (11:39 +0100)
Sweep logic currently does not kick in until the root statement of
a particular source is completely built. This leads to us retaining
all of the state until that moment, at which point we mass-release
everything.

This is caused by us being needlessly conservative in deciding when
to start sweeping our parents.

Replace ReactorStmtCtx.noParentRefs() with noParentRefcount(), which
disregards noImplicitRef(). This allows us to start sweeping as
soon as we buildEffective() any child of a subtree, propagating
towards parent -- which is safe, since there can be no new incoming
explicit references.

JIRA: YANGTOOLS-1184
Change-Id: I97feeb65c270b1e7abceb99b94938593cf2e9abc
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
yang/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ReactorStmtCtx.java

index 18fbf9e918819e0539d0f4ff523ab1b7f7ceb1ac..0de3e281e3a9c7f459057c4e034a902fd0087fe5 100644 (file)
@@ -179,10 +179,9 @@ abstract class ReactorStmtCtx<A, D extends DeclaredStatement<A>, E extends Effec
         if (parent == null) {
             // We are the top-level object and have lost a reference. Trigger sweep if possible and we are done.
             sweepState();
-            return;
+        } else {
+            parent.sweepOnChildDecrement();
         }
-
-        parent.sweepOnChildDecrement();
     }
 
     // Called from child when it has lost its final reference
@@ -201,28 +200,28 @@ abstract class ReactorStmtCtx<A, D extends DeclaredStatement<A>, E extends Effec
         }
 
         // parent is potentially reclaimable
-        if (noParentRefs()) {
+        if (noParentRefcount()) {
             LOG.trace("Cleanup {} of parent {}", refcount, this);
-            sweepState();
+            if (sweepState()) {
+                final ReactorStmtCtx<?, ?, ?> parent = parentStmtCtx();
+                if (parent != null) {
+                    parent.sweepOnChildDecrement();
+                }
+            }
         }
     }
 
     // FIXME: cache the resolution of this
-    private boolean noParentRefs() {
+    private boolean noParentRefcount() {
         final ReactorStmtCtx<?, ?, ?> parent = parentStmtCtx();
         if (parent != null) {
+            // There are three possibilities:
+            // - REFCOUNT_NONE, in which case we need to search next parent
+            // - negative (< REFCOUNT_NONE), meaning parent is in some stage of sweeping, hence it does not have
+            //   a reference to us
+            // - positive (> REFCOUNT_NONE), meaning parent has an explicit refcount which is holding us down
             final int refs = parent.refcount;
-            // FIXME: 'noImplicitRef' is too strict here?
-            if (refs > REFCOUNT_NONE || !parent.noImplictRef()) {
-                // parent with refcount or protected by views
-                return false;
-            }
-            if (refs < REFCOUNT_NONE) {
-                // parent is being swept already
-                return true;
-            }
-            // REFCOUNT_NONE and reclaimable, look forward
-            return parent.noParentRefs();
+            return refs == REFCOUNT_NONE ? parent.noParentRefcount() : refs < REFCOUNT_NONE;
         }
         return true;
     }
@@ -249,15 +248,11 @@ abstract class ReactorStmtCtx<A, D extends DeclaredStatement<A>, E extends Effec
         LOG.trace("Child refcount {}", refcount);
         if (refcount == REFCOUNT_NONE) {
             sweepDone();
-            sweepParent();
-        }
-    }
-
-    private void sweepParent() {
-        final ReactorStmtCtx<?, ?, ?> parent = parentStmtCtx();
-        LOG.trace("Propagating to parent {}", parent);
-        if (parent != null && parent.isAwaitingChildren()) {
-            parent.sweepOnChildDone();
+            final ReactorStmtCtx<?, ?, ?> parent = parentStmtCtx();
+            LOG.trace("Propagating to parent {}", parent);
+            if (parent != null && parent.isAwaitingChildren()) {
+                parent.sweepOnChildDone();
+            }
         }
     }