From 4974cabb8123853fd5c8fb6113d95cd58c6a1ec3 Mon Sep 17 00:00:00 2001
From: Robert Varga <robert.varga@pantheon.tech>
Date: Tue, 1 Dec 2020 23:33:10 +0100
Subject: [PATCH] Sweep child->parent relationships more quickly

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>
---
 .../parser/stmt/reactor/ReactorStmtCtx.java   | 47 +++++++++----------
 1 file changed, 21 insertions(+), 26 deletions(-)

diff --git a/yang/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ReactorStmtCtx.java b/yang/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ReactorStmtCtx.java
index 18fbf9e918..0de3e281e3 100644
--- a/yang/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ReactorStmtCtx.java
+++ b/yang/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ReactorStmtCtx.java
@@ -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();
+            }
         }
     }
 
-- 
2.36.6