From bc8cd94d3d317d783963524e63842310bc206ab1 Mon Sep 17 00:00:00 2001
From: Robert Varga <robert.varga@pantheon.tech>
Date: Sun, 28 May 2023 22:04:12 +0200
Subject: [PATCH] Relax AbstractDataObjectModification.childNodesCache

This is quite a critical part of computation: use
getAcquire()/setRelease() mechanics. There is a twist, since the object
is an ImmutableList -- use compare-and-swap to ensure concurrent loads
result in the same list.

Change-Id: I4ea9773f415cd03bbba2e8068ce54367dd1d46de
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
---
 .../AbstractDataObjectModification.java       | 31 ++++++++++---------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/AbstractDataObjectModification.java b/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/AbstractDataObjectModification.java
index 9346e9a617..023ed75e5d 100644
--- a/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/AbstractDataObjectModification.java
+++ b/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/AbstractDataObjectModification.java
@@ -61,6 +61,7 @@ abstract sealed class AbstractDataObjectModification<T extends DataObject, N ext
         permits LazyAugmentationModification, LazyDataObjectModification {
     private static final Logger LOG = LoggerFactory.getLogger(AbstractDataObjectModification.class);
     private static final VarHandle MODIFICATION_TYPE;
+    private static final VarHandle MODIFIED_CHILDREN;
 
     static {
         final var lookup = MethodHandles.lookup();
@@ -68,6 +69,8 @@ abstract sealed class AbstractDataObjectModification<T extends DataObject, N ext
         try {
             MODIFICATION_TYPE = lookup.findVarHandle(AbstractDataObjectModification.class, "modificationType",
                 ModificationType.class);
+            MODIFIED_CHILDREN = lookup.findVarHandle(AbstractDataObjectModification.class, "modifiedChildren",
+                ImmutableList.class);
         } catch (NoSuchFieldException | IllegalAccessException e) {
             throw new ExceptionInInitializerError(e);
         }
@@ -77,7 +80,8 @@ abstract sealed class AbstractDataObjectModification<T extends DataObject, N ext
     final @NonNull PathArgument identifier;
     final @NonNull N codec;
 
-    private volatile ImmutableList<AbstractDataObjectModification<?, ?>> childNodesCache;
+    @SuppressWarnings("unused")
+    private volatile ImmutableList<AbstractDataObjectModification<?, ?>> modifiedChildren;
     @SuppressWarnings("unused")
     private volatile ModificationType modificationType;
     private volatile @Nullable T dataBefore;
@@ -178,11 +182,8 @@ abstract sealed class AbstractDataObjectModification<T extends DataObject, N ext
 
     @Override
     public final ImmutableList<AbstractDataObjectModification<?, ?>> getModifiedChildren() {
-        var local = childNodesCache;
-        if (local == null) {
-            childNodesCache = local = createModifiedChilden(codec, domData, domChildNodes());
-        }
-        return local;
+        final var local = (ImmutableList<AbstractDataObjectModification<?, ?>>) MODIFIED_CHILDREN.getAcquire(this);
+        return local != null ? local : loadModifiedChilden();
     }
 
     @Override
@@ -199,6 +200,16 @@ abstract sealed class AbstractDataObjectModification<T extends DataObject, N ext
             .collect(Collectors.toList());
     }
 
+    @SuppressWarnings("unchecked")
+    private @NonNull ImmutableList<AbstractDataObjectModification<?, ?>> loadModifiedChilden() {
+        final var builder = ImmutableList.<AbstractDataObjectModification<?, ?>>builder();
+        populateList(builder, codec, domData, domChildNodes());
+        final var computed = builder.build();
+        // Non-trivial return: use CAS to ensure we reuse concurrent loads
+        final var witness = MODIFIED_CHILDREN.compareAndExchangeRelease(this, null, computed);
+        return witness == null ? computed : (ImmutableList<AbstractDataObjectModification<?, ?>>) witness;
+    }
+
     @SuppressWarnings("unchecked")
     private <C extends DataObject> Stream<LazyDataObjectModification<C>> streamModifiedChildren(
             final Class<C> childType) {
@@ -282,14 +293,6 @@ abstract sealed class AbstractDataObjectModification<T extends DataObject, N ext
         };
     }
 
-    private static @NonNull ImmutableList<AbstractDataObjectModification<?, ?>> createModifiedChilden(
-            final CommonDataObjectCodecTreeNode<?> parentCodec, final DataTreeCandidateNode parent,
-            final Collection<DataTreeCandidateNode> children) {
-        final var result = ImmutableList.<AbstractDataObjectModification<?, ?>>builder();
-        populateList(result, parentCodec, parent, children);
-        return result.build();
-    }
-
     private static void populateList(final ImmutableList.Builder<AbstractDataObjectModification<?, ?>> result,
             final CommonDataObjectCodecTreeNode<?> parentCodec, final DataTreeCandidateNode parent,
             final Collection<DataTreeCandidateNode> children) {
-- 
2.36.6