cloud-fan commented on code in PR #58865:
URL: https://github.com/apache/spark/pull/58865#discussion_r4129480283


##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/trees/TreeNodeSuite.scala:
##########
@@ -98,7 +104,50 @@ case class FakeLeafPlan(child: LogicalPlan)
 
 case class FakeCurryingProduct(x: Expression)(val y: Int)
 
+case class ConcurrentTreeNode(barrier: CyclicBarrier, computeCount: 
AtomicInteger)
+  extends LeafNode {
+  override def output: Seq[Attribute] = Nil
+
+  override protected def getDefaultTreePatternBits: BitSet = {
+    computeCount.incrementAndGet()
+    barrier.await(10, TimeUnit.SECONDS)
+    new BitSet(TreePattern.maxId)
+  }
+}
+
 class TreeNodeSuite extends SparkFunSuite with SQLHelper {
+  test("constructing unused Literals does not allocate eager cache state") {
+    val literal = Literal(1)
+    val cacheFields = classOf[TreeNode[_]].getDeclaredFields
+      .filter(_.getName.endsWith("CacheValue"))
+
+    assert(cacheFields.length === 5)
+    cacheFields.foreach { field =>
+      field.setAccessible(true)
+      assert(field.get(literal) === null, s"${field.getName} was eagerly 
initialized")
+    }
+  }
+
+  test("concurrent cache initialization returns and reuses the selected 
value") {
+    val barrier = new CyclicBarrier(2)
+    val computeCount = new AtomicInteger
+    val node = ConcurrentTreeNode(barrier, computeCount)
+    val executor = ThreadUtils.newDaemonFixedThreadPool(2, 
"tree-node-cache-test")
+    val executionContext = ExecutionContext.fromExecutorService(executor)
+    try {
+      val first = Future(node.treePatternBits)(executionContext)

Review Comment:
   **Non-blocking (P2):** The post-construction test only calls 
`treePatternBits`, even though this patch hand-wires five separate field, 
VarHandle, and accessor paths. For example, swapping the same-typed 
`heightCache` and `hashCodeCache` handles, or dropping publication for a 
collection cache, still leaves both new tests passing while height/hash results 
can be crossed or collections rebuilt on every call.
   
   **Verification:**
   - **Regression:** Exercise every cache accessor on a controlled tree, verify 
the expected value and corresponding field population, repeat the access, and 
assert that the cached object is retained; the checks must fail if the integer 
VarHandles are swapped or any collection accessor omits publication.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to