dwsmith1983 commented on code in PR #5615:
URL: https://github.com/apache/datafusion-comet/pull/5615#discussion_r3921083630
##########
spark/src/main/scala/org/apache/spark/sql/comet/operators.scala:
##########
@@ -328,6 +378,32 @@ private[comet] object IcebergPlanDataInjector extends
PlanDataInjector {
* Injector for NativeScan operators.
*/
private[comet] object NativeScanPlanDataInjector extends PlanDataInjector {
+ import java.nio.ByteBuffer
+ import java.util.{LinkedHashMap, Map => JMap}
+
+ private final val maxCacheEntries = 16
+
+ // Same rationale as IcebergPlanDataInjector's commonCache: the common bytes
are identical
+ // for every partition of a stage, and parsing them dominates inject() for
wide schemas.
+ private val commonCache = java.util.Collections.synchronizedMap(
+ new LinkedHashMap[ByteBuffer, OperatorOuterClass.NativeScanCommon](4,
0.75f, true) {
+ override def removeEldestEntry(
+ eldest: JMap.Entry[ByteBuffer,
OperatorOuterClass.NativeScanCommon]): Boolean = {
+ size() > maxCacheEntries
Review Comment:
Done. The cache value is now a holder with the parsed plan plus its prepared
per scan commons, so everything for a plan lives and dies with its single
entry. Reran your churn shapes: 1 plan with 17 scans goes from 0/17 reused to
17/17, nine plans with two scans each from 0/18 to 18/18, and the warm pass
drops from 14.7ms to 1.0ms. Numbers in the main comment.
--
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]
