olabusayoT commented on code in PR #1717:
URL: https://github.com/apache/daffodil/pull/1717#discussion_r3836361274
##########
daffodil-core/src/main/scala/org/apache/daffodil/runtime1/processors/SuspensionTracker.scala:
##########
@@ -46,16 +98,67 @@ class SuspensionTracker(suspensionWaitYoung: Int,
suspensionWaitOld: Int) {
* evaluate are moved to the old suspensions list. If we evaluate old
* suspensions, we attempt to evaluate them first, with the hope that their
* resolution might make the young suspensions more likely to evaluate.
+ *
+ * skipLengthStateWaiters = true here: a suspension with
+ * isWaitingOnLengthState true has a targeted wake-up already
+ * registered (fired from CaptureEndOf{Content,Value}LengthUnparsers
+ * once its length becomes computable) and can't progress until that
+ * fires - retrying it on the blind periodic schedule first is pure
+ * wasted DPath re-evaluation.
*/
- def evalSuspensions(): Unit = {
+ def evalSuspensions(): Unit =
+ evalSuspensionsThrottled(filterToBuildResolvable = false,
skipLengthStateWaiters = true)
+
+ /**
+ * A discard-sink sweep variant: same throttled cadence as
+ * evalSuspensions, but passes filterToBuildResolvable=true to
+ * evalSuspensionQueue. A suspension whose canResolveWithoutWriting is
+ * false can never be satisfied by a discard-sink traversal no matter
+ * how many retries, so it's skipped-and-requeued instead of really
+ * attempted - unless it's already isWaitingOnLengthState, in which
+ * case it's parked instead (parking only ever follows one of the real
+ * sweep's (evalSuspensions) own unfiltered attempts having set that
+ * flag; this filtered sweep never sets it itself). Either way the
+ * suspension stays pending for that real sweep's later, unfiltered
+ * attempts once real bytes exist for it to depend on.
+ *
+ * Eliminates the wasted doTask cost of this discard-sink sweep for
+ * these suspensions; doesn't eliminate the smaller per-tick
+ * dequeue/requeue cost for suspensions with no targeted wake-up at all
+ * (e.g. padding/target-length SuspendableOperations), which must stay
+ * on the skip-and-requeue path so the real sweep still finds them.
+ *
+ * skipLengthStateWaiters is left false here: canResolveWithoutWriting
Review Comment:
Unfortunately some test in our current test rig break with the proposed
"lengthstate suspensions never evaluate until notified" plan, because of DOS
Splitting/merging, so in those situations, the registered wakeup never gets
fired. Here's is a more details explanation from claude
Here's what's actually going on in TestOutputValueCalc1's
OutputValueCalc_01:
Trace (OutputValueCalc_01: x needs valueLength(y), y needs valueLength(z)):
```
first-attempt SuspendableExpression(x, valueLength(y)) → isDone=false (y
unwritten)
first-attempt SimpleTypeRetryUnparser(x) → isDone=false (x has no value)
first-attempt SuspendableExpression(y, valueLength(z)) → isDone=false (z
unwritten)
first-attempt SimpleTypeRetryUnparser(y) → isDone=false (y has no value)
retry SuspendableExpression(x) isWaitingOnLengthState=false
registerWaiter valueLen(y) <- SuspendableExpression(x) ← x registers its
ONE targeted wake-up, on y's length
retry-result SuspendableExpression(x) → isDone=false
retry SimpleTypeRetryUnparser(x) → isDone=false
retry SuspendableExpression(y) → isDone=true (z is a plain string, already
known)
retry SimpleTypeRetryUnparser(y) → isDone=true (y's bytes now written)
retry SimpleTypeRetryUnparser(x) → isDone=false (still no value for x)
retry SimpleTypeRetryUnparser(x) → isDone=false
retry SuspendableExpression(x) isWaitingOnLengthState=true ← STILL
registered, no notify ever fired
retry-result SuspendableExpression(x) → isDone=true ← succeeds anyway, via
a plain re-attempt
```
The point: x registers exactly once, against y's ValueLengthState. y's
length never triggers that registered wake-up as y's length only becomes
computable as a side effect of y's own SimpleTypeRetryUnparser succeeding (a
completely different suspension object, blocked on y's value, not on any length
at all) plus the surrounding DOS-splitting machinery converging. Grepping the
run for the wake-up call (moveFromParkedToYoung, the only path a registered
notify can take) and for the notify-check itself found zero hits. x succeeds
purely because something keeps giving it a plain, unconditional re-attempt, not
because anything ever told it "your wait is over."
So "never evaluate a suspension until it's notified" is unsound as a
general design: x's only completion path here is a blind retry with a stale,
never-fired registration still sitting on it. A purely event-driven model has
no event to catch this, which is exactly why the periodic retry has to stay.
--
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]
