Interop: a JavaScript function converts to the delegate type the host asked for - #3521
Conversation
sebastienros#3521 claimed 4.82 while this was in flight. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S
* Temporal: adding months in hebrew counts the Metonic cycle hebrew was the walk #3519 left behind, and so was every other calendar the engine reads a BCL Calendar for: add({ months: n }) asked each year in turn how many months it held, and past the end of that calendar's own table it asked by catching the ArgumentOutOfRangeException the call raises there. A month difference walks additions, so it was quadratic — eighteen thousand years of hebrew took 71 seconds. The year range each backing calendar answers for is now read once from its own MinSupportedDateTime and MaxSupportedDateTime and compared against; only GetDaysInMonth and ToDateTime keep a catch, because a year inside a band can still hold a month it declines. And the month a step lands on is arithmetic: the Metonic cycle for hebrew, a division by the year length for a calendar whose years are all the same length. The year walk stays as the fallback for the landing year no int can hold, and for a calendar a host ICalendarProvider answers for. No date moved: the two reckonings agree on all 483,875 compared cases. Fixes #3520 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S * Docs: renumber the migration section to 4.83 #3521 claimed 4.82 while this was in flight. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
a587c93 to
0a386bf
Compare
|
Discharging the "gate tables to follow" promise with the measurement decision instead of tables, per the gating session's findings: The gated suites cannot reach this lane. SunSpider and Dromaeo are pure-JS workloads; no case in either converts a JavaScript function to a CLR delegate, so a paired run would measure noise and report it as evidence. What can be said by inspection is stronger here: the hit path is strictly cheaper than before — the same Merging on correctness (7 reproductions, both orders, shared- |
…sions and operators (backport of #3521 and #3526) (#3559) Three process-wide interop caches were keyed on less than the answer they hold depends on, so whichever engine reached one first decided for every engine after it. `DefaultTypeConverter` memoizes the CLR delegate a JavaScript function converts to - it has to, since `host.on(f)` and `host.off(f)` only pair up if both conversions hand the host the same `Delegate` instance. The memo was keyed on the function instance, and one level below it on that function's AST node. Neither carries the target delegate `Type`, which is the one thing `BuildTargetBinderDelegate` bakes in, so the second target type a function was converted for was served the first one's delegate and the reflection invoke behind the call rejected it. Both caches are type-keyed now, through an append-only `TypeKeyedCache<T>` published with one `Interlocked.CompareExchange`, since a shared `Prepared<Script>` is documented as usable from several engines at once. The AST node is process-wide state that outlives every engine that ran the preparation, and the binder it held baked the *converting* engine in as a constant. So a second engine running the same preparation marshalled its host callback's arguments through the first engine's realm - an `instanceof Object` that answers `false` - and kept that engine, its realm and its intrinsics alive for the life of the AST. The binder reads the engine off the target function instead, which is what makes the entry shareable at all. `JintBinaryExpression._knownOperators` remembers which CLR operator a `(name, left type, right type)` triple selects, and the selection runs `InteropHelper.FindBestMatch`, which reads two things the embedder configures: `Options.Interop.ValueCoercion`, which the gray-zone scoring rule consults, and the installed `ITypeConverter`, whose answer is the last rule outright. The coercion setting is a value, so it goes in the key and engines that share it go on sharing entries; the converter is a host object the engine's factory was handed, so keying a process-lived static on it would pin that converter and its engine forever - it selects the table instead, an engine with its own converter keeping its resolutions on itself where they die with it. Fixes #3434 and #3424 on 4.x.
Fixes #3434.
DefaultTypeConvertermemoizes the CLR delegate a JavaScript function converts to, and it has to:host.on(f)followed by
host.off(f)only unsubscribes if both conversions hand the host the sameDelegateinstance.The memo was keyed on the function instance, and one level below it on that function's AST node. Neither
carries the target delegate
Type, which is the one thingBuildTargetBinderDelegate(type, func)bakes intothe compiled binder — so the second target type a given function was converted for was served the first one's
delegate, and the reflection invoke behind the call rejected it.
The premise still holds on today's
main(f6fa93bb7).The three reproductions, as failing tests against unmodified
mainJint.Tests/Runtime/DelegateConversionTargetTypeTests.cs— four cases, of whichControlEachHostAlone(each host alone, in its own engine) passes before and after and is there to say the conversion itself was
never broken:
The first is the bound-delegate cache (one function instance, two delegate types, one engine); the second is
the binder cache below it (two instances of one AST node); the third is the reason the cache exists at all —
the same function converted twice to the same delegate type must be the same instance — asserted per target
type rather than for whichever type asked first.
And the sharp one, in
Jint.Tests.PublicInterface/HostDelegateConversionTests.cs, because the AST node isprocess-wide state and
Engine.PrepareScriptis documented as shareable across engines. It needs no seconddelegate type in any one script: two engines, different host types, one cached
Prepared<Script>— the shapethe README recommends for production — and whichever evaluated first decided for both.
The other-order case is there so the fix cannot be a rule about which type wins, and the alternating one so it
cannot be a single slot that the last engine through overwrites.
The key shape, and why this one
Three shapes were on the table. A composite key is not available:
ConditionalWeakTablekeys on referenceidentity, so a
(function, Type)key object would never match a later one and would root the function itnames — the weak-key semantics are the whole reason these are
ConditionalWeakTables.The #3501 value-keyed memo — store the
Typethe entry was built for and rebuild on a mismatch — is rightwhere the rebuild is cheap, and this rebuild is not. There it re-adapts a regex through
RegExpParseCache, adictionary lookup, so two engines alternating on one node is a non-event; here it is
Expression.Compile()ona freshly built expression tree. A single slot would recompile a binder on every alternation of the third
reproduction, which is exactly the shape this PR is about.
So: a per-entry, type-keyed map, on both caches.
TypeKeyedCache<T>is an append-only singly-linked listof
(Type, T)nodes, read with aVolatile.Readand published with oneInterlocked.CompareExchange— thebinder cache genuinely can be reached concurrently, since a shared
Prepared<Script>is documented asthread-safe across engines. Entries are added and never replaced, so the identity guarantee survives and is
merely finer-grained: one
Delegateper (function instance, delegate type), which is exactly the identity-=needs, where one per function instance was only accidentally it.Cost shape on the hit path
No benchmark numbers here — this is a warm interop path and the machine is busy. Gate tables to follow in a
comment before merge. What the change does to the shape:
ConditionalWeakTableprobe returning aDelegate, it is now the same probe returning the per-entry mapplus a walk of a list that is one node long for the overwhelming majority of functions, which are
converted to exactly one delegate type. That walk is one reference compare against the node's
Type.ConditionalWeakTable.GetValue(key, callback)needs its callbackconstructed before the probe, and the old callback closed over
type,funcandfunctionInstance— adisplay class and a delegate allocated on every conversion, hit or miss. The callbacks are now cached
static lambdas that capture nothing. The
_hostCallbackDelegatesregistration beside it gets the sametreatment: it probes first and only builds its closure when there is something to add.
the extra cost is one small node object beside the
Expression.Compile()that dominates it.total, which is one for essentially every function and two for the deliberately mixed case. The function
instance holds one delegate per target type, on the same terms.
Lifetimes
Both tables stay
ConditionalWeakTable, so entries still die with the node or the object they hang off, andnothing new is rooted statically. What the AST node now carries — a
Type, and a binder that takes its targetas a parameter — is engine-neutral, as
Jint/Runtime/Interpreter/AGENTS.mdrequires of anything reachablefrom a shared tree.
GarbageCollectionTestsgainsSharedPreparedScriptConvertingFunctionsToDelegatesDoesNotRetainEngines: twenty engines convert two functionsto two delegate types through one shared preparation, and every engine must be collectable while the
preparation stays rooted. It is a pin rather than a reproduction — it passes on
maintoo, and would have to,since the point is that the fix did not make the shared cache engine-affine. The existing
SharedPreparedScriptDoesNotRetainEngines, its parse-only siblings andNestedTypeAccessDoesNotRetainEnginesall pass (11/11 in that fixture).
Test results
Jint.Testsnet472 / net8.0 / net10.0Jint.Tests.PublicInterfacenet472 / net8.0 / net10.0Jint.Tests.PublicInterfacenet8.0,JINT_HOST_CONTRACT_VERIFICATION=1Jint.Tests.CommonScripts,Jint.Tests.SourceGeneratorsJint.Tests.Test262The suite total is the expected 102,688 and nothing fails. The skip count is four above the figure this branch
was measured against; all 151 skips resolve to entries in
Test262Harness.settings.json, so they areconfiguration rather than run-time timeouts, and neither that file nor the pinned SHA is touched here — the
difference is
maindrift since that reading, and this change reaches no test262 case, none of which convertsa JavaScript function to a CLR delegate.
dotnet build -c Releaseon the solution is clean withTreatWarningsAsErrors. No public signature changed,so no API baselines were regenerated.
Documentation
Migration guide: §4.82 — the wrong-delegate
ArgumentExceptiondisappearing is observable, even thoughnothing that used to succeed changes. The shared-cache rule in
Jint/Runtime/Interpreter/AGENTS.md, whichalready says a shared interop cache may hold only engine-independent values, gains the other half of it: a
shared cache's key must carry everything its value was built for.