Repository navigation
Refresh async memory accounting for current runtime - #3216
Conversation
|
Heads-up on the rebase this now needs: the merged #3036 carries one thing that was not in the version you refreshed from.
That is precisely what this PR does, so on rebase those call sites should move to the three-argument accounted overload, and the unaccounted one can go with them if nothing is left using it. Keeping it around after this lands would just be a documented hole nobody needs. Everything else in #3036 is as you had it; the only other change from the refresh was composing |
Carry allocation state through the pump-driven web continuations, preserve persistent-event task boundaries, coordinate transferred callback threads, and abort transient work safely when a memory operation fails. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0d07275e-227c-4672-becd-4fe336386da4
The rebase across the eighteen commits since #3036 leaves five things that had to change, plus two defects the refresh introduced. Reconciliation. `ConstraintFailure.MustPropagate` composes over `Throw.MustPropagateHostException` on main, and that list already carries a marker — `Throw.TimeoutException` and `Throw.OperationCanceledException` register the exception they mint in `EngineAbortRegistry`, so Jint's own timeout and cancellation are already distinguishable from a host's. The refresh brought a second marker of its own (an `Exception.Data` key, `Throw.Constraint*Exception` duplicates of three throw helpers, a `MemoryLimitPlatformNotSupportedException` subclass) and a standalone list to read it with. All of that is gone; what it was for is kept, in the one place it was actually needed: the faulted-Task scan in `JsValue.ConvertTaskToPromise` reads `Throw.MustPropagateHostException` rather than `ConstraintFailure.MustPropagate`, because for a queued turn a `TimeoutException` is Jint's own and nothing else, while for a host Task it is an `HttpClient` timeout and must stay a promise rejection. `Constraint.BeforeFailure` is gone too: the same refresh added `Engine.CheckConstraint`, which does the same job at the call site and covers user-derived constraints the hook could never reach. It is now a `try` around each of the three constraint loops rather than a wrapper per constraint, so the non-throwing path is unchanged instruction for instruction, and the filter excludes `JavaScriptException` — script can catch that one, and abandoning the operation under it would hand the script a fresh budget. The five `AddToEventLoop` overloads collapse to three: `EventLoopRegistration` now carries every deferred registration, so `(Action, int, OperationState?)` and `EnqueueModuleLoadCompletion`'s triple go away, and `ModuleLoadCompletion` and the `Atomics.waitAsync` waiter each lose a field. `HostStreamBridge` hands the registration back from `TryBeginOperation` instead of parking it in a field for the next line to read, and its now-unread `Generation` goes with it. `GetHostCallbackOwner` no longer returns an owner, so it is `GetHostCallbackAuthorization`. Defects. A `setInterval` inherited the budget of the operation that scheduled it and charged every tick to it, so an engine whose heartbeat allocates a few bytes a second died after enough ticks — the refresh's own rule says repeating work starts fresh, and its docs said timers were finite continuations. A repeating `TimerEntry` now keeps the cycle stamp and drops the budget, which is what every persistent event source already does; `AnIntervalStartsAFreshBudgetOnEveryTick` fails without it. And `MemoryLimitConstraint`'s teardown runs host code — a worker host told its connection ended, a host stream disposed — which may re-enter the engine and find a fresh operation over the same budget, so it is guarded to run once. Two tests join it: an engine is usable again after a teardown, and a teardown discards the timers the failed operation scheduled. Co-authored-by: Sébastien Ros <1165805+sebastienros@users.noreply.github.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VecfHbij4EMQFzL7NCpdWW
234cb6b to
5cc597f
Compare
|
Rebased onto What main had grown under this
On the
|
| suite | result |
|---|---|
Jint.Tests |
10,052 net10.0 / 7,102 net472, 0 failed |
Jint.Tests + Jint.Tests.PublicInterface, JINT_HOST_CONTRACT_VERIFICATION=1 |
0 failed |
Jint.Tests.PublicInterface |
2,395 net10.0 / 1,874 net472, 0 failed |
Jint.Tests.CommonScripts |
28 + 28, 0 failed |
Jint.Tests.SourceGenerators |
52, 0 failed |
Jint.Tests.Test262 |
102,488 passed, 7 failed — see below |
The seven are staging/sm/TypedArray/{element-setting-converts-using-ToNumber,sort_large_countingsort}.js, ArrayBuffer.prototype.sliceToImmutable/argument-coercion.js and Atomics/notify/notify-renotify-noop.js, all of them 30-second engine timeouts on a loaded machine. main at 9817530 fails the same set on the same box — 22 of them in one full run, and 3 of 3 when the subset is run alone against each branch in turn. No delta.
Not benchmarked. Two things touch a hot path and both look free by inspection: HostCallScope grows two fields whose only writer is the transferred-callback constructor (SROA'd, constant branch in Dispose), and the constraint loops gain a try region that emits nothing until it throws. Say the word if you want a gate run on EngineConstructionBenchmark / EngineComparisonBenchmark anyway.
|
CI is green on The first I reproduced the deadline one on both trees anyway rather than assume, by pinning the test host to two logical CPUs under contention: Test262 on the CI runner: 102,495 passed, 0 failed. (The seven I reported locally were all 30 s engine timeouts on a loaded box, and |
…ider (#3268) `TimerQueue` has taken its clock from a `TimeProvider` since #3216, which is what lets its tests drive it with a `ManualClock` and assert exactly. `TimeConstraint` and `OperationDeadlineConstraint` read `Stopwatch.GetTimestamp()` directly, so nothing could move their clock and every test of them was a real-time test — one-sided at best, plus a skip for the run where a contended machine stalled past the whole budget. That is the shape behind #3123, #3154, #3221 and the sighting in #3230; each was fixed by widening or by weakening the claim rather than by removing the cause. The seam is public, because the problem is not Jint's alone: any embedder that configures `TimeoutInterval` has the same untestable wall clock. Two entry points, matching who creates the constraint — `Options.Constraints.TimeProvider` for the one the engine registers, and an `OperationDeadlineConstraint(TimeProvider)` overload for the one the host constructs itself. Both are `net8.0`-and-later: reaching `TimeProvider` from `net462`/`netstandard2.x` would mean a second runtime dependency on a package that has exactly one, which is not a trade worth making for a seam whose main consumer is a test. Cost on the amortized lane, which is where `TimeConstraint.Check` is driven: * `ConstraintClock.Resolve` folds `TimeProvider.System` — and no provider — to a null field, and a null field means the constraint reads `Stopwatch` directly exactly as before. `TimeProvider`'s own base `GetTimestamp` and `TimestampFrequency` *are* `Stopwatch`'s, so the fold changes no answer. * Default-path codegen gains three instructions: a load from the same cache line as `_deadline`, a test, and a never-taken branch, in the shadow of an ~18 ns `QueryPerformanceCounter`. Measured per check across six rounds of a monomorphic micro-benchmark: |delta| <= 0.26 ns, best-conditioned rounds ~0.1 ns. * `Check` runs once per 64 statements. On the most check-dense row in the suite (`ConstrainedExecutionBenchmark`'s 1M-iteration loop) that is exactly 15,625 checks per evaluation, so the delta is <= 4.1 us of a ~20.4 ms evaluation — <= 0.02% of the row, and <= 0.8% of what the whole amortized timeout lane already costs it (~2.4%). Two orders of magnitude below what a paired BenchmarkDotNet A/B can resolve, which is why the evidence here is codegen plus an isolated per-check measurement rather than a suite A/B. * On the downlevel targets the field does not exist and neither does the branch: the only change is a static `AggressiveInlining` helper that inlines to the instruction it replaced. No constraint behaviour changes. Same exception types, same messages, same firing points, same `IsAmortizable`, same no-op `Reset` on the operation deadline; the interval-to-ticks arithmetic and its overflow clamp move to `ConstraintClock.ToTimestampTicks` unaltered. A clock reporting a non-positive `TimestampFrequency` is rejected where it is supplied, since it would otherwise arm every deadline at its own start and fail the first check of every execution. The payoff, on `HostResultLimitsTests.OperationDeadlineSpansEvaluationAndConversion`: it used to spend the budget with a 500 ms `Thread.Sleep` and could then make only two claims — "the evaluation did not throw", skipped when a stalled runner made it throw anyway, and "the conversion did throw" — because how much of a wall-clock budget an entry consumes is a fact about the machine. It now straddles the budget to the tick: one tick short of it a fresh entry still completes, and the tick that exhausts it fails the next one. Neither half was expressible before, there is no skip and no sleep, and everything except the clock is the shipped path. The downlevel leg keeps the old row. Both halves were confirmed live by mutation: arming `Begin` off the seam, and arming the deadline one tick early, each fail it. Verified: solution builds Release on all five target frameworks; `Jint.Tests` (10498/7256) and `Jint.Tests.PublicInterface` (2490/1899) green on both target frameworks, plain and with `JINT_HOST_CONTRACT_VERIFICATION=1`; test262 102495 passed / 189 skipped / 102684 total, identical to an `upstream/main` baseline run on the same machine. Fixes #3232 Claude-Session: https://claude.ai/code/session_01FHnbgQGW5QgfatxL6DgGEd Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Summary
Context
Follow-up to merged #3036. This is the semantic current-
mainrefresh requested in the cascade review comment. The latestQuotaExceededError, pump-driven timeout, ownership, WPT and web-API changes remain intact.Validation
dotnet build Jint/Jint.csproj -c Releasegit diff --checkBenchmark project restore is currently blocked by the mandated package proxy not carrying
YantraJS.Core1.2.462 (nearest available 1.2.422); no alternate feed was used.