Store cached child els in Attributes, not field (#2307)

We keep a lazy initialized, cached copy of an Element's child Element list, to save recreating it every time they are requested.

That was stored in a nullable field. This change moves it into the userdata section of the attributes instead.

Also, rather than tracking changes to the backing node list via a ChangeNotifyingArrayList, I'm using the ArrayList's modcount instead directly. So we can deprecate the ChangeNotifyingArrayList.

Ultimately this trims the shallow size of an Element from 40 to 32 bytes, and the NodeList from 32 to 24 (20% and 25% decreases respectively)

Author: jhy

src/main/java/org/jsoup/helper/ChangeNotifyingArrayList.java

@@ -5,8 +5,9 @@
 
 /**
  * Implementation of ArrayList that watches out for changes to the contents.
+ @deprecated this class is no longer used and will be removed in jsoup 1.21.1.
  */
-public abstract class ChangeNotifyingArrayList<E> extends ArrayList<E> {
+@Deprecated public abstract class ChangeNotifyingArrayList<E> extends ArrayList<E> {
     public ChangeNotifyingArrayList(int initialCapacity) {
         super(initialCapacity);
     }

src/main/java/org/jsoup/nodes/Element.java

@@ -1,6 +1,5 @@
 package org.jsoup.nodes;
 
-import org.jsoup.helper.ChangeNotifyingArrayList;
 import org.jsoup.helper.Validate;
 import org.jsoup.internal.Normalizer;
 import org.jsoup.internal.StringUtil;
@@ -37,7 +36,6 @@
 import java.util.stream.Stream;
 
 import static org.jsoup.internal.Normalizer.normalize;
-import static org.jsoup.nodes.Document.OutputSettings.Syntax.html;
 import static org.jsoup.nodes.Document.OutputSettings.Syntax.xml;
 import static org.jsoup.nodes.TextNode.lastCharIsWhitespace;
 import static org.jsoup.parser.Parser.NamespaceHtml;
@@ -50,11 +48,11 @@ An HTML Element consists of a tag name, attributes, and child nodes (including t
 */
 public class Element extends Node implements Iterable<Element> {
     private static final List<Element> EmptyChildren = Collections.emptyList();
+    private static final NodeList EmptyNodeList = new NodeList(0);
     private static final Pattern ClassSplit = Pattern.compile("\\s+");
     private static final String BaseUriKey = Attributes.internalKey("baseUri");
     Tag tag;
-    private @Nullable WeakReference<List<Element>> shadowChildrenRef; // points to child elements shadowed from node children
-    List<Node> childNodes;
+    NodeList childNodes;
     @Nullable Attributes attributes; // field is nullable but all methods for attributes are non-null
 
     /**
@@ -86,7 +84,7 @@ public Element(String tag) {
      */
     public Element(Tag tag, @Nullable String baseUri, @Nullable Attributes attributes) {
         Validate.notNull(tag);
-        childNodes = EmptyNodes;
+        childNodes = EmptyNodeList;
         this.attributes = attributes;
         this.tag = tag;
         if (baseUri != null)
@@ -108,12 +106,12 @@ public Element(Tag tag, @Nullable String baseUri) {
      Internal test to check if a nodelist object has been created.
      */
     protected boolean hasChildNodes() {
-        return childNodes != EmptyNodes;
+        return childNodes != EmptyNodeList;
     }
 
     @Override protected List<Node> ensureChildNodes() {
-        if (childNodes == EmptyNodes) {
-            childNodes = new NodeList(this, 4);
+        if (childNodes == EmptyNodeList) {
+            childNodes = new NodeList(4);
         }
         return childNodes;
     }
@@ -393,31 +391,40 @@ public Elements children() {
      * @return a list of child elements
      */
     List<Element> childElementsList() {
-        if (childNodeSize() == 0)
-            return EmptyChildren; // short circuit creating empty
-
-        List<Element> children;
-        if (shadowChildrenRef == null || (children = shadowChildrenRef.get()) == null) {
-            final int size = childNodes.size();
-            children = new ArrayList<>(size);
-            //noinspection ForLoopReplaceableByForEach (beacause it allocates an Iterator which is wasteful here)
-            for (int i = 0; i < size; i++) {
-                final Node node = childNodes.get(i);
-                if (node instanceof Element)
-                    children.add((Element) node);
-            }
-            shadowChildrenRef = new WeakReference<>(children);
+        if (childNodeSize() == 0) return EmptyChildren; // short circuit creating empty
+        List<Element> children = cachedChildren();
+        if (children == null) {
+            children = filterNodes(Element.class);
+            stashChildren(children);
         }
         return children;
     }
 
-    /**
-     * Clears the cached shadow child elements.
-     */
-    @Override
-    void nodelistChanged() {
-        super.nodelistChanged();
-        shadowChildrenRef = null;
+    private static final String childElsKey = "jsoup.childEls";
+    private static final String childElsMod = "jsoup.childElsMod";
+
+    /** returns the cached child els, if they exist, and the modcount of our childnodes matches the stashed modcount */
+    private @Nullable List<Element> cachedChildren() {
+        Map<String, Object> userData = attributes().userData();
+        //noinspection unchecked
+        WeakReference<List<Element>> ref = (WeakReference<List<Element>>) userData.get(childElsKey);
+        if (ref != null) {
+            List<Element> els = ref.get();
+            if (els != null) {
+                Integer modCount = (Integer) userData.get(childElsMod);
+                if (modCount != null && modCount == childNodes.modCount())
+                    return els;
+            }
+        }
+        return null;
+    }
+
+    /** caches the child els into the Attribute user data. */
+    private void stashChildren(List<Element> els) {
+        Map<String, Object> userData = attributes().userData();
+        WeakReference<List<Element>> ref = new WeakReference<>(els);
+        userData.put(childElsKey, ref);
+        userData.put(childElsMod, childNodes.modCount());
     }
 
     /**
@@ -1898,7 +1905,7 @@ public Element shallowClone() {
     protected Element doClone(@Nullable Node parent) {
         Element clone = (Element) super.doClone(parent);
         clone.attributes = attributes != null ? attributes.clone() : null;
-        clone.childNodes = new NodeList(clone, childNodes.size());
+        clone.childNodes = new NodeList(childNodes.size());
         clone.childNodes.addAll(childNodes); // the children then get iterated and cloned in Node.clone
 
         return clone;
@@ -1961,16 +1968,13 @@ public Element filter(NodeFilter nodeFilter) {
         return  (Element) super.filter(nodeFilter);
     }
 
-    private static final class NodeList extends ChangeNotifyingArrayList<Node> {
-        private final Element owner;
-
-        NodeList(Element owner, int initialCapacity) {
-            super(initialCapacity);
-            this.owner = owner;
+    static final class NodeList extends ArrayList<Node> {
+        public NodeList(int size) {
+            super(size);
         }
 
-        @Override public void onContentsChanged() {
-            owner.nodelistChanged();
+        int modCount() {
+            return this.modCount;
         }
     }
 }

src/main/java/org/jsoup/nodes/Node.java

@@ -501,10 +501,6 @@ private static Element getDeepChild(Element el) {
         return el;
     }
 
-    void nodelistChanged() {
-        // Element overrides this to clear its shadow children elements
-    }
-
     /**
      * Replace this node in the DOM with the supplied node.
      * @param in the node that will replace the existing node.