From 3bb55b9d652a2bdf2e789c1f717ee97429f9e026 Mon Sep 17 00:00:00 2001
From: olabusayoT <50379531+olabusayoT@users.noreply.github.com>
Date: Fri, 2 Oct 2026 18:57:28 -0400
Subject: [PATCH] Inline MStack operations and allow sizing stacks up front

MStack's @specialized annotation has no effect under Scala 3, so push, pop, setTop and top on the Boolean, Int, Long and reference stacks boxed the element and went through ScalaRunTime's type-dispatching array_update and array_apply. Make push, pop, setTop, top and bottom inline, so each call site is expanded with the stack's concrete element type and compiles to direct array loads and stores.

Stacks take an initialSize, 32 by default, so a caller that knows its depth can avoid growing the array. A diagnostic, off unless MStack.trackMaxSizeReached is set, records the deepest a stack has grown, for choosing a size. TestMStack covers the operations, growth and copyFrom.

DAFFODIL-3065
---
 .../org/apache/daffodil/lib/util/MStack.scala |  92 ++++++++-------
 .../apache/daffodil/lib/util/TestMStack.scala | 106 ++++++++++++++++++
 2 files changed, 157 insertions(+), 41 deletions(-)

diff --git a/daffodil-core/src/main/scala/org/apache/daffodil/lib/util/MStack.scala b/daffodil-core/src/main/scala/org/apache/daffodil/lib/util/MStack.scala
index 6c25a867bb..e0243700d2 100644
--- a/daffodil-core/src/main/scala/org/apache/daffodil/lib/util/MStack.scala
+++ b/daffodil-core/src/main/scala/org/apache/daffodil/lib/util/MStack.scala
@@ -24,6 +24,12 @@ import Maybe.*
 object MStack {
   final case class Mark(val v: Int) extends AnyVal
   val nullMark = Mark(0)
+
+  /**
+   * Off by default: growing past initialSize isn't itself wrong, so paying
+   * this bookkeeping cost on every push isn't worth it normally.
+   */
+  final val trackMaxSizeReached: Boolean = false
 }
 
 /**
@@ -33,35 +39,25 @@ object MStack {
  * catches improper initialization. These were not initializing properly,
  * so the idiom evolved to use the scala initializers.
  */
-final class MStackOfBoolean private ()
-  extends MStack[Boolean]((n: Int) => new Array[Boolean](n), false)
+final class MStackOfBoolean private (initialSize: Int)
+  extends MStack[Boolean]((n: Int) => new Array[Boolean](n), false, initialSize)
 
 object MStackOfBoolean {
-  def apply() = {
-    val stk = new MStackOfBoolean()
-    stk.init()
-    stk
-  }
+  def apply(initialSize: Int = 32) = new MStackOfBoolean(initialSize)
 }
 
-final class MStackOfInt extends MStack[Int]((n: Int) => new Array[Int](n), 0)
+final class MStackOfInt(initialSize: Int)
+  extends MStack[Int]((n: Int) => new Array[Int](n), 0, initialSize)
 
 object MStackOfInt {
-  def apply() = {
-    val stk = new MStackOfInt()
-    stk.init()
-    stk
-  }
+  def apply(initialSize: Int = 32) = new MStackOfInt(initialSize)
 }
 
-final class MStackOfLong extends MStack[Long]((n: Int) => new Array[Long](n), 0L)
+final class MStackOfLong(initialSize: Int)
+  extends MStack[Long]((n: Int) => new Array[Long](n), 0L, initialSize)
 
 object MStackOfLong {
-  def apply() = {
-    val stk = new MStackOfLong()
-    stk.init()
-    stk
-  }
+  def apply(initialSize: Int = 32) = new MStackOfLong(initialSize)
 }
 
 /**
@@ -75,11 +71,11 @@ object MStackOfLong {
  * So we use an Array[AnyRef] as the representation here, and we
  * convert null to Nope, and an actual object reference to One(x)
  */
-final class MStackOfMaybe[T <: AnyRef] {
+final class MStackOfMaybe[T <: AnyRef](initialSize: Int = 32) {
 
   override def toString = delegate.toString
 
-  private val delegate = new MStackOf[T]
+  private val delegate = new MStackOf[T](initialSize)
   private val nullT = null.asInstanceOf[T]
 
   def copyFrom(other: MStackOfMaybe[T]) = delegate.copyFrom(other.delegate)
@@ -120,6 +116,7 @@ final class MStackOfMaybe[T <: AnyRef] {
   def toListMaybe = delegate.toList.map { (x: AnyRef) =>
     Maybe(x) // Scala compiler bug without this cast
   }
+  def maxSizeReached = delegate.maxSizeReached
 }
 
 /**
@@ -135,7 +132,7 @@ final class MStackOfMaybe[T <: AnyRef] {
  * an object reference or null, and call Maybe(thing) explicitly outside the
  * iteration. Maybe(null) is Nope, and Maybe(thing) is One(thing) if thing is not null.
  */
-final class MStackOf[T <: AnyRef] extends Serializable {
+final class MStackOf[T <: AnyRef](initialSize: Int = 32) extends Serializable {
 
   override def toString = delegate.toString
 
@@ -143,7 +140,7 @@ final class MStackOf[T <: AnyRef] extends Serializable {
 
   @inline final def length = delegate.length
 
-  private val delegate = MStackOfAnyRef()
+  private val delegate = MStackOfAnyRef(initialSize)
 
   @inline final def mark = delegate.mark
   @inline final def reset(m: MStack.Mark) = delegate.reset(m)
@@ -156,6 +153,7 @@ final class MStackOf[T <: AnyRef] extends Serializable {
   @inline final def isEmpty = delegate.isEmpty
   def clear() = delegate.clear()
   def toList = delegate.toList
+  def maxSizeReached = delegate.maxSizeReached
 
   def iterator = delegate.iterator.asInstanceOf[ResettableIterator[T]]
 
@@ -163,15 +161,15 @@ final class MStackOf[T <: AnyRef] extends Serializable {
 
 }
 
-private[util] final class MStackOfAnyRef private ()
-  extends MStack[AnyRef]((n: Int) => new Array[AnyRef](n), null.asInstanceOf[AnyRef])
+private[util] final class MStackOfAnyRef private (initialSize: Int)
+  extends MStack[AnyRef](
+    (n: Int) => new Array[AnyRef](n),
+    null.asInstanceOf[AnyRef],
+    initialSize
+  )
 
 object MStackOfAnyRef {
-  def apply() = {
-    val stk = new MStackOfAnyRef()
-    stk.init()
-    stk
-  }
+  def apply(initialSize: Int = 32) = new MStackOfAnyRef(initialSize)
 }
 
 /**
@@ -184,16 +182,22 @@ object MStackOfAnyRef {
  */
 protected abstract class MStack[@specialized T] private[util] (
   arrayAllocator: (Int) => Array[T],
-  nullValue: T
+  nullValue: T,
+  initialSize: Int = 32
 ) {
 
   private var index = 0
-  private var table: Array[T] = null
+  private var table: Array[T] = arrayAllocator(initialSize)
 
-  def init(): Unit = {
-    index = 0
-    table = arrayAllocator(32)
-  }
+  private var maxSizeReached_ = 0
+
+  /**
+   * The largest this stack's length has ever grown to, across its whole
+   * lifetime (pops don't reduce it). Diagnostic only, for checking whether
+   * an initialSize is well-chosen. Always 0 unless
+   * MStack.trackMaxSizeReached is enabled.
+   */
+  final def maxSizeReached: Int = maxSizeReached_
 
   def copyFrom(other: MStack[T]): Unit = {
     this.index = other.index
@@ -212,6 +216,9 @@ protected abstract class MStack[@specialized T] private[util] (
       }
 
     }
+    if (MStack.trackMaxSizeReached && other.maxSizeReached_ > this.maxSizeReached_) {
+      this.maxSizeReached_ = other.maxSizeReached_
+    }
   }
   // private var currentIteratorIndex = -1
 
@@ -250,10 +257,13 @@ protected abstract class MStack[@specialized T] private[util] (
    *
    *  @param x The element to push
    */
-  @inline final def push(x: T): Unit = {
+  inline def push(x: T): Unit = {
     if (index == table.length) table = growArray(table)
     table(index) = x
     index += 1
+    if (MStack.trackMaxSizeReached && index > maxSizeReached_) {
+      maxSizeReached_ = index
+    }
   }
 
   /**
@@ -261,7 +271,7 @@ protected abstract class MStack[@specialized T] private[util] (
    *
    *  @return the element on top of the stack
    */
-  @inline final def pop(): T = {
+  inline def pop(): T = {
     if (index == 0) Assert.usageError("Stack empty")
     index -= 1
     val x = table(index)
@@ -275,7 +285,7 @@ protected abstract class MStack[@specialized T] private[util] (
    *
    * @param x The element to set to the top of the stack
    */
-  @inline final def setTop(x: T): Unit = {
+  inline def setTop(x: T): Unit = {
     if (index == 0) Assert.usageError("Stack empty")
     table(index - 1) = x
   }
@@ -288,9 +298,9 @@ protected abstract class MStack[@specialized T] private[util] (
    *
    *  @return the element on top of the stack.
    */
-  @inline final def top: T = table(index - 1)
+  inline def top: T = table(index - 1)
 
-  @inline final def bottom: T = table(0)
+  inline def bottom: T = table(0)
 
   @inline final def isEmpty: Boolean = index == 0
 
diff --git a/daffodil-core/src/test/scala/org/apache/daffodil/lib/util/TestMStack.scala b/daffodil-core/src/test/scala/org/apache/daffodil/lib/util/TestMStack.scala
index b4cebcdbcb..5905e6deb3 100644
--- a/daffodil-core/src/test/scala/org/apache/daffodil/lib/util/TestMStack.scala
+++ b/daffodil-core/src/test/scala/org/apache/daffodil/lib/util/TestMStack.scala
@@ -17,6 +17,11 @@
 
 package org.apache.daffodil.lib.util
 
+import org.apache.daffodil.lib.util.Maybe.*
+
+import org.junit.Assert.*
+import org.junit.Test
+
 /**
  * Compare MStack performance to ArrayStack. It should be faster for primitives
  */
@@ -24,6 +29,107 @@ class TestMStack {
 
   var junk: Long = 0
 
+  // Copying into a sized destination must match copying into a default one.
+  @Test def testMStackOfCopyFromSizedDestinationMatchesDefault(): Unit = {
+    val source = new MStackOf[String]
+    source.push("a")
+    source.push("b")
+    source.push("c")
+
+    val sizedDest = new MStackOf[String](source.length)
+    sizedDest.copyFrom(source)
+
+    val defaultDest = new MStackOf[String]
+    defaultDest.copyFrom(source)
+
+    assertEquals(defaultDest.toList, sizedDest.toList)
+    assertEquals(3, sizedDest.length)
+    assertEquals("c", sizedDest.top)
+  }
+
+  // A sized-to-exact-depth destination must still grow past capacity.
+  @Test def testMStackOfSizedDestinationStillGrowsCorrectly(): Unit = {
+    val source = new MStackOf[String]
+    source.push("a")
+    source.push("b")
+
+    val sizedDest = new MStackOf[String](source.length)
+    sizedDest.copyFrom(source)
+
+    sizedDest.push("c")
+    sizedDest.push("d")
+    sizedDest.push("e")
+
+    assertEquals(5, sizedDest.length)
+    assertEquals("e", sizedDest.top)
+    assertEquals(List("e", "d", "c", "b", "a"), sizedDest.toList)
+  }
+
+  /** Same sized-clone pattern, but for MStackOfMaybe. */
+  @Test def testMStackOfMaybeCopyFromSizedDestinationMatchesDefault(): Unit = {
+    val source = new MStackOfMaybe[String]
+    source.push(One("x"))
+    source.push(Nope)
+    source.push(One("z"))
+
+    val sizedDest = new MStackOfMaybe[String](source.length)
+    sizedDest.copyFrom(source)
+
+    val defaultDest = new MStackOfMaybe[String]
+    defaultDest.copyFrom(source)
+
+    assertEquals(defaultDest.toListMaybe, sizedDest.toListMaybe)
+    assertEquals(3, sizedDest.length)
+    assertEquals(One("z"), sizedDest.top)
+    assertEquals(One("z"), sizedDest.pop)
+    assertEquals(Nope, sizedDest.pop)
+    assertEquals(One("x"), sizedDest.pop)
+    assertTrue(sizedDest.isEmpty)
+  }
+
+  /** Same sized-construction pattern, for the primitive-specialized variants. */
+  @Test def testMStackOfBooleanSizedConstructionWorks(): Unit = {
+    val stk = MStackOfBoolean(3)
+    stk.push(true)
+    stk.push(false)
+    stk.push(true)
+    assertEquals(3, stk.length)
+    assertEquals(true, stk.pop())
+    assertEquals(false, stk.pop())
+    assertEquals(true, stk.pop())
+  }
+
+  @Test def testMStackOfIntSizedConstructionWorks(): Unit = {
+    val stk = MStackOfInt(2)
+    stk.push(1)
+    stk.push(2)
+    stk.push(3)
+    assertEquals(3, stk.length)
+    assertEquals(3, stk.pop())
+    assertEquals(2, stk.pop())
+    assertEquals(1, stk.pop())
+  }
+
+  @Test def testMStackOfLongSizedConstructionWorks(): Unit = {
+    val stk = MStackOfLong(1)
+    stk.push(10L)
+    stk.push(20L)
+    assertEquals(2, stk.length)
+    assertEquals(20L, stk.pop())
+    assertEquals(10L, stk.pop())
+  }
+
+  // trackMaxSizeReached is a `final val`; confirms it's off by default
+  // and maxSizeReached stays 0 while it is.
+  @Test def testMaxSizeReachedDisabledByDefault(): Unit = {
+    assertEquals(false, MStack.trackMaxSizeReached)
+    val stk = new MStackOf[String]
+    stk.push("a")
+    stk.push("b")
+    stk.push("c")
+    assertEquals(0, stk.maxSizeReached)
+  }
+
   /**
    * This test compares MStackOfLong to ArrayStack[Long].
    *