Upgrade Jint to 4.15.3 and adopt its cache-gate and lazy-value APIs - #1326
Conversation
The Jint upgrade that follows re-arms three engine-wide inline cache gates. Each of those changes is only worth making if it is observably identical, so pin the behaviour first, against Jint 4.8.0: - null propagation: undeclared identifier reads, nullish property chains, calls over a nullish base, calls of a non-callable member (which return the base), and that ordinary member reads/calls are unaffected; - enum values crossing into script as their member name, including a [Flags] combination and an enum member of a wrapped CLR object; - JSON objects projected into script: own key order, JSON.stringify, for..in, mutation (add/replace/delete) and the round trip back to JsonValue; - content field objects: `in`, hasOwnProperty, propertyIsEnumerable, Object.keys, spread, JSON.stringify, and that all of them follow deletes and additions; - the context object: key enumeration, `in`, typeof per value, reads, write-through to ScriptVars and delete. All 156 tests in Operations/Scripting pass unchanged on 4.8.0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016uV6H9cTntzsoKiaJRBn4f
Package provenance verified after restore: source is https://api.nuget.org/v3/index.json and the nuspec repository commit is a304aa5dacd340e2a5ff51e1ea0c465e38e50aa8, the v4.15.3 tag. Acornima moves to 1.6.2 transitively; ParseErrorException, ScriptPreparationException and JintException, which JintScriptEngine.MapException switches on, all still exist, and so do Engine.Constraints.Reset, Options.Constraints.PromiseTimeout, AllowClrWrite, EvaluateAsync(in Prepared<Script>, CancellationToken), ObjectWrapper.Create and the ObjectInstance virtuals the ContentWrapper family overrides. No source change is needed for the upgrade itself: the whole backend solution builds warning-clean and all 156 tests in Operations/Scripting, including the behaviour pins added in the previous commit, pass unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016uV6H9cTntzsoKiaJRBn4f
The scripting integration defines several Jint extension points: the ContentWrapper objects override GetOwnProperty, and the engine trusts the answer without re-verifying it on the hot path. A hook that contradicts another therefore fails silently in production - a key vanishes from every enumeration, or a read resolves on the prototype for a property that exists - which is the class of bug no assertion in this repository would catch. Jint 4.15.3 exposes its host-contract verifiers to the shipped Release package through an AppContext switch, where before they were compiled out unless you built the engine from source in Debug. A module initializer sets it for this test assembly, so the verifiers run against the same NuGet package production uses and report a violation as an ordinary test failure. It must be set before the first use of any Jint type, which is exactly what a module initializer guarantees. Confirmed live rather than assumed: with a deliberately wrong ProbeOwnProperty the run fails with "ContentFieldObject.ProbeOwnProperty answered 'iv' with Missing but its GetOwnProperty reports Enumerable". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016uV6H9cTntzsoKiaJRBn4f
Registering an IReferenceResolver without interests gives it ReferenceResolverInterests.All, and two of those flags - ObjectPropertyBase and PrimitivePropertyBase - are the gate on the non-computed member-read inline caches, the dense-array indexed-read lane and the member-call callee lane. With All declared, every property read in every script has to be routed through a Reference so the resolver gets offered the base, and all three lanes stay off for the whole engine. NullPropagation.TryPropertyReference returns false for every base that is not null or undefined, so on those two situations the engine consulting it can never change the result. Declaring only the three situations the resolver actually answers - NullishPropertyBase, UnresolvableReference and NonCallableCallee - is therefore observably identical and re-arms all three lanes. Interests are documented as a subscription filter and not a promise: a situation that is not subscribed to behaves exactly as if no resolver were registered. Jint also ships a built-in NullPropagatingReferenceResolver, which is deliberately NOT adopted here: it declines unresolvable identifiers and non-callable callees, where this resolver answers both, so swapping it in would turn an undeclared-name read and a call on a nullish chain into errors for existing tenant scripts. The behaviour pins from the first commit cover exactly those edges and all 156 tests in Operations/Scripting still pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016uV6H9cTntzsoKiaJRBn4f
An IObjectConverter registered without declaring the CLR types it handles can be handed anything, so the engine has to assume every wrapped CLR member read might reach it and disables the compiled interop member-read lane engine-wide. JintObjectConverter handles a closed set, so declare it: matching is by assignability, which keeps IUser covering every implementation. The converter is still offered every value - the declaration only lets the engine keep the fast lane for members whose declared type could never produce a handled value, and it errs towards claiming (a member typed `object` is always claimed). The Enum branch is dropped in favour of Options.Interop.EnumConversion = EnumConversionMode.String, which Jint documents as the member name "as produced by object.ToString()", including the comma-separated combination for a [Flags] value and the numeric value rendered as a string for a value with no name - verbatim what the branch did. The write direction keeps accepting both the name and the number. Handling enums natively rather than through the converter also keeps one more declared type off the list, so more members stay on the fast lane. Pinned by Should_convert_enum_to_name, Should_convert_flags_enum_to_names and Should_convert_enum_member_of_wrapped_object_to_name, which pass before and after. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016uV6H9cTntzsoKiaJRBn4f
JsonMapper built every JSON object it projects into script as an instance of a private ObjectInstance subclass that existed only to be instantiable. A host subclass can never carry the engine's shape-mode storage flag, so each of those objects - all of ctx.data's leaf objects, and every JsonValue var - sat permanently outside the own-property inline caches, and a script reading the same property across a batch of content items re-resolved it every time. JsObject.CreateFromEntries builds the same object through the hidden class machinery instead: repeated calls presenting the same key sequence, which every content item of one schema does, share an interned hidden class, so those reads stay monomorphic. The result is documented as indistinguishable from the equivalent object literal - same own key order, same configurable/enumerable/writable data properties - and anything the representation cannot express (a digit-leading key, a very wide object) falls back to the ordinary dictionary representation rather than to different behaviour. That fallback is silent, which is why the shaping is asserted rather than assumed: Engine.Advanced.HasSharedShape is the supported predicate for it, and JsonMapperTests pins that the projected object and its nested objects answer true. Building them as a host subclass again would fail that test. Three smaller fixes in the same file: - the reverse direction allocated a string key per array element (a.Get(i.ToString(...))); the indexed accessor reads the dense backing directly and keeps the prototype walk for a modified array; - JsNumber.Create reuses cached instances for small integers where new JsNumber always allocated; - JsString.Create, public since 4.15.3, interns the empty and single character strings where new JsString always allocated. Pinned by the projection tests added first - own key order, JSON.stringify, for..in, mutation including delete and add, and the round trip back to JsonValue - which pass before and after. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016uV6H9cTntzsoKiaJRBn4f
ContentFieldProperty is a CustomJsValue descriptor whose CustomValue maps the stored JsonValue to a JsValue on first read. Existence and enumerability questions never need that value, but they used to pay for it: `in`, hasOwnProperty, propertyIsEnumerable, Object.keys/values/entries, Object.assign, object spread and JSON.stringify all reached the object through GetOwnProperty, which materializes the descriptor whose value the caller then reads or discards. Jint lets a host answer those questions directly through ProbeOwnProperty. The override deliberately mirrors GetOwnProperty line for line, minus the descriptor: same initialization, same toJSON exclusion, same lookup, and the enumerable flag read off the descriptor rather than off its value. The engine trusts the probe without re-verifying it on the hot path, so a wrong Missing would silently drop the key from every enumeration above - which is why the two are kept adjacent in the file, pinned by tests covering `in`, hasOwnProperty, propertyIsEnumerable, Object.keys, spread and JSON.stringify plus a delete and an add, and checked on every test run by the host-contract verification enabled earlier in this branch. ContentDataObject deliberately does not get the same override: its GetOwnProperty auto-creates a field for any name probed, so a probe that agreed with it at the same instant would have to do the same, and that quirk is pre-existing tenant-visible behaviour this change has no business altering. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016uV6H9cTntzsoKiaJRBn4f
Every variable was mapped when the engine was set up, once per evaluation, whether or not the script ever looked at it - WritableContext did it in its constructor for the ctx path, and Engine.SetValue did it per variable for the non-context path. Some of those mappings are not cheap: a user variable walks and groups every claim, a content data variable builds a wrapper. A typical script reads a handful of the variables available to it. Both paths now defer the mapping to the first read of the value, through the two APIs Jint 4.15.3 added for exactly this: - PropertyDescriptor.CreateLazy for the ctx object. Unlike a hand-written CustomJsValue descriptor it drops the flag once the value exists, so the descriptor rejoins the write inline cache instead of paying the indirection for the rest of its life. - Engine.Advanced.AddLazyGlobal for the non-context path. The options-time AddLazyGlobal could not serve it - the variables are only known after the engine has been built - and the descriptor a host could install itself is declined by the global-identifier cache. The Advanced overload is documented as being for exactly this case, and its factory may capture engine-affine state. In both cases the property itself is installed eagerly, so nothing about the shape changes: key order, enumeration, `in`, Object.getOwnPropertyNames, delete and the write-through to ScriptVars behave exactly as before, which is what the tests pin - including a counting principal that proves the mapping has not run for a variable the script never mentions, and has run for one it reads. MapVariable reproduces Engine.SetValue's special case for a CLR type so a deferred variable cannot project differently. One edge is worth recording: Engine.SetValue writes through [[Set]] while AddLazyGlobal replaces the descriptor, so a variable named after a non-writable built-in global (undefined, NaN, Infinity) would now shadow it where it was previously ignored. ScriptVars keys are domain names, so this is not reachable in practice. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016uV6H9cTntzsoKiaJRBn4f
| for (var i = 0u; i < length; i++) | ||
| { | ||
| result.Add(Map(a.Get(i.ToString(CultureInfo.InvariantCulture)))); | ||
| result.Add(Map(a[i])); |
There was a problem hiding this comment.
No idea what this means.
There was a problem hiding this comment.
It's just a simpler way to index array adirectly without creating a temporary string to create numeric index which Jint then has to transform to number again.
|
Thanks a lot, I have no idea what most of the comments actually mean, so I have to go over the PR manually: I think the whole PR is really digging a lot into the irrelevant details of Jint and is hard to understand. OR |
|
The shapes feature is the "hidden shapes", so basically when we know the form of objects we don't need to create expensive and slower dictionaries with property descriptors. If you just want the basic speed improvement, updating the library will do. It's the B→C part that is the interop optimization with Jint's new features. |
|
It is really awesome, what you have achieved with AI. The number of PRs in jint is great. I just wish that the agent would create "normal" comments when creating PRs outside of Jint. For me it seems it uses the internal domain language and comments in a way that nobody really understands. Could you instruct the agent to write the comments in a more condensed and easy to understand form? For example // Sets a value, but runs the conversion only when the value is used for the first time |
The comments explained the change in Jint's own vocabulary - inline caches, shape mode, descriptors, lanes - which is not vocabulary this repository uses. Say what each change does and why it is worth it instead, and name a Jint concept only where the reader has to look it up anyway. No behaviour change: comments and XML docs only, plus one short comment on the enum conversion option.
|
Fair point, and thanks for saying it directly — I rewrote both. Comments are one or two lines each now ( // Sets the value, but runs the conversion only when the script reads it for the first time.
// The name is added right away, so enumeration and "in" checks work as before.
engine.Advanced.AddLazyGlobal(key, e => MapVariable(e, item));The rule I applied: say what it does and why it is worth it, and name a Jint concept only where you would have to look it up anyway — then explain it right there. I kept it as a separate commit instead of folding it into the other eight, so that whatever you already reviewed manually stays valid. Description now leads with the two independent parts (the version bump, which needs no code change, and the six adoption changes), then a table of what each change does in one sentence. There is a short section up front for the three terms that are hard to avoid — inline cache, shape, host — with an example from our code for each; they come from how JS engines are built (V8, SpiderMonkey), not from anything Squidex-specific. All the deep justification, edge cases and the compatibility checklist are in collapsed sections now. No code changed, and the numbers are the same. |
Upgrades Jint from 4.8.0 to 4.15.3. There are two independent parts:
.csproj. No source change needed, everything builds and all scripting tests pass unmodified. Scripts get 2–39% faster on their own, depending on what they do.Scripts behave exactly the same in both parts — this PR is only about speed.
Sorry for the wall of engine jargon in the first version of this description. This one tries again.
The terms I keep using
They come from how JavaScript engines are built (V8, SpiderMonkey, and by now Jint too):
Inline cache. When a script does
ctx.data.title.iv, the engine remembers at that exact spot in the script where it foundtitlelast time, and reuses that on the next run instead of searching again. It can only do that when nothing is able to change the answer behind its back. If we register a hook and do not tell Jint what the hook is for, Jint has to assume the worst and switches these caches off — for the whole engine, not just for the objects we hooked. Two of our registrations did exactly that.Shape (V8 calls it a hidden class). Objects that are created with the same keys can share one description of their layout, like a class, instead of every object carrying its own property dictionary. All content items of one schema have the same keys, so this applies to almost all JSON we push into scripts. A shared layout is cheaper to read from, and it is what makes the inline cache above possible in the first place. Objects built from a custom
ObjectInstancesubclass — which is howJsonMapperbuilt them — can never take part in this.Host. Jint's word for the application that embeds it, so: us. A host contract is a place where Jint calls into our code and then trusts the answer without verifying it, because verifying would cost as much as the shortcut saves.
ContentDataObjectand friends are such places.What the six changes are
NullPropagation.csJintObjectConverter.csJsonMapper.csContentFieldObject.cs'iv' in fieldwithout converting the field value.WritableContext.cs,JintExtensions.csNumbers
Measured on an idle machine with BenchmarkDotNet, strictly serial, three checkouts sharing an identical harness: A =
masteron 4.8.0, B =masterwith only the version line changed, C = this PR. Every scenario asserts its exact expected output before being measured, and all three arms produced the same outputs.JSON.stringify(data)ctx(change 5)Two things worth being honest about:
Allocations are down in every scenario compared to today.
The changes in detail
1. Tell Jint what
NullPropagationdoesSetReferencesResolverwithout a second argument means "this resolver might react to anything", and Jint answers by disabling its member-read caches engine-wide, for every property read in every script.NullPropagation.TryPropertyReferencereturnsfalsefor everything that is not null or undefined, so for the other cases Jint asking us can never change the result. Declaring that re-enables the caches:Jint documents this list as a subscription filter and not a promise: for a case we do not subscribe to, it behaves as if no resolver was registered at all — which is what it would have done anyway.
Why not the built-in
NullPropagatingReferenceResolverthat 4.15 ships?Because it is not the same behavior. It does not answer unresolvable names or non-callable members, where our resolver does, so swapping it in would turn
unknownNameandctx.value.notAFunction()into errors for existing scripts. Two tests pin that difference.One oddity found on the way and left alone: our
TryUnresolvableReferencepasses the reference base straight through, and for an unknown name that base is an internal Jint marker string reading[[Unresolvable]], notundefined. SoString(unknownName)has always been"[[Unresolvable]]"here. Changing it toundefinedwould be tidier but is a behavior change for existing scripts, so it is only documented and pinned by a test.2. Tell Jint which types
JintObjectConverterhandlesSame shape of problem: a converter that has not declared its types might be handed anything, so Jint has to offer it every .NET property read and cannot use its compiled property reader for any of them. The converter handles a closed set of nine types, now declared as
JintObjectConverter.HandledTypesand kept directly above theswitchit must match.Matching is by assignability, so
typeof(IUser)still covers every implementation, and the filter errs towards claiming — the converter is still offered every value it was offered before.The
Enumcase is dropped in favor ofInterop.EnumConversion = EnumConversionMode.String, which Jint documents as the member name as produced byToString()— including"ContentScript, Transform"for a[Flags]combination and the number as a string for a value with no name. Same output as the branch it replaces, tests for all three cases.The obvious risk is that someone adds a
casehere and forgets the list. That is not silent: change 6 makes it fail a test by name (verified by deliberately breaking it).3. Let
JsonMapperbuild shared-layout objectsJsonMapperbuilt every JSON object as a privateObjectInstancesubclass that existed only to be instantiable — and such an object can never share a layout, so all ofctx.data's leaf objects and every JSON variable were permanently on the slow path.JsObject.CreateFromEntriesbuilds the same object the way Jint builds an object literal — same keys, same order, same flags, indistinguishable from a script — but through the layout machinery, so repeated calls with the same key sequence share one layout. That is the JSON walk row above (−38% of it from this change alone).Jint falls back to the per-object dictionary silently when it cannot build the shared form, so
JsonMapperTestsasserts it actually happened (HasSharedShape) rather than assuming it.Three unrelated small fixes in the same file: the reverse direction allocated a string per array element (
a.Get(i.ToString())) where the indexer reads the array storage directly, andJsNumber.Create/JsString.Createreuse cached instances wherenewalways allocated.4.
ContentFieldObjectanswers existence questions without convertingContentFieldPropertyconverts the storedJsonValueon first read. Questions like'iv' in field,hasOwnProperty,Object.keys, spread andJSON.stringifydo not need the value at all, but used to pay for it, because they all went throughGetOwnProperty. The newProbeOwnPropertyanswers them from the property flags instead.It is deliberately a copy of
GetOwnPropertyminus the value, because Jint trusts it without checking — a wrong answer would silently drop keys from every enumeration above. Change 6 is what checks it.ContentDataObjectdeliberately does not get the same treatment; see below.5. Convert script variables on first read
Every variable was converted when the engine was set up, whether the script mentioned it or not — and some are not cheap (the user variable walks and groups all claims). A typical script uses two or three of them.
Both paths now defer that, with the two APIs 4.15 added for it:
The property itself is added immediately in both cases, so key order,
Object.keys,in,getOwnPropertyNames,deleteand the write-through toScriptVarsare unchanged — only the conversion waits.MapVariablereproduces whatEngine.SetValuedoes, including its special case for aType, so a deferred variable cannot look different from an eager one. Six tests use aClaimsPrincipalthat counts how often its claims are read to prove all three properties: not converted when unread, still visible in enumeration, converted on first read.One edge case worth recording
Engine.SetValueassigns through the normal write path whileAddLazyGlobalinstalls the property directly, so a variable named exactly like a non-writable built-in (undefined,NaN,Infinity) would now shadow it where it was silently ignored before.ScriptVarskeys are domain names, so this cannot happen in practice.6. Jint's self-checks run in the test suite
Changes 1, 2 and 4 all have the same failure mode: we tell Jint something, Jint believes us without checking, and if we are wrong nothing throws — a key just disappears from
Object.keys, or a type stops being converted.4.15 can turn those checks on in the shipped release package through an
AppContextswitch (before, this needed a source build of Jint in Debug). A module initializer in the test assembly sets it, so every CI run verifies our extension points against the same NuGet package production uses, and a violation is an ordinary test failure. It stays off in production, where the checks would only cost performance.Both checks were confirmed by deliberately breaking them first:
ProbeOwnPropertyfails with "ContentFieldObject.ProbeOwnProperty answered 'iv' with Missing but its GetOwnProperty reports Enumerable";With both correct, all 1279 tests in
Squidex.Domain.Apps.Core.Testspass with the checks on.What I deliberately did not do
ScriptExecutionContext's scheduler re-enters the engine from task continuations underlock (Engine), guarded only byIsCompleted, and on timeoutWaitForCompletionAsyncreturns while theTaskCompletionSourceis neither completed nor faulted — so a latescheduler.Runcan re-enter the engine afterwards. Today that engine is garbage and nobody notices. Pooled, it would write one evaluation's data into the next one's variables. That needs a kill flag in the scheduler first, and is its own PR.ContentDataObject.GetOwnPropertycreating a field for any name it is asked about. Pre-existing and visible to scripts (data.hasOwnPropertyreads as a field rather than the function, and asking about a name makes it show up inObject.keys). Not this PR's business — but it is also the reasonContentDataObjectgets noProbeOwnPropertyin change 4: to stay consistent with thatGetOwnProperty, the probe would have to create phantom fields too.Options.AddImmutableCrossing(new in 4.15, lets Jint cache reads of .NET objects a host promises will not change while scripts can see them). Does not fit:ScriptVarsis written through by design andAssetMetadatais mutated by scripts on purpose, there is a test for it. It is also an options-time setting, and the types that would be candidates live inSquidex.Domain.Apps.Entities, which the engine setup cannot reference. A wrong promise here means stale reads, so this is not a place to guess.SetTypeConverter. It is needed (extension delegates takeJsonValueparameters). Worth recording that it costs something: with a custom type converter installed, Jint excludes indexer accessors from its shared cross-engine cache. If that ever shows up in a profile, the fix is a sharedTypeResolverfor all Squidex engines, not removing the converter.Tests
backend/tests/Squidex.Domain.Apps.Core.Tests/Operations/Scripting/goes from 128 to 164 tests.Everything behavior-adjacent was pinned before the change and run green on the old code first: the first commit adds 28 tests and passes on Jint 4.8.0, which is bisectable. They cover undeclared names, null-propagating chains, calls on non-callable members, ordinary reads and calls, enum conversion including
[Flags], JSON object key order /stringify/for..in/ mutation / round trip, content fields underin/hasOwnProperty/propertyIsEnumerable/Object.keys/ spread /stringify(also after a delete and an add), and the context object's enumeration,typeof, write-through anddelete.The remaining 8 arrive with the features they describe and are red before the change, green after.
Verification runs
Run with the same commands CI uses (
Dockerfile:dotnet test --filter "Category!=Dependencies & Category!=TestContainer" --configuration Release), so Meziantou.Analyzer and StyleCop ran in-build. One StyleCop diagnostic was introduced and fixed properly (SA1203); no suppressions added.Squidex.Domain.Apps.Core.Tests1279/1279, including all 164 scripting tests, with Jint's self-checks on.Squidex.Infrastructure.Tests1073/1073 ·Squidex.Domain.Users.Tests57/57 ·Squidex.Web.Tests167/167.Squidex.Domain.Apps.Entities.Tests1524/1526 — the two failures areFFMpegAssetMetadataSourceTests, which need theffmpegbinary that the CI image has and my machine does not. They touch no scripting code.The first eight commits are logical units and each builds and passes on its own. The ninth rewrites the code comments in plainer language after review feedback and changes nothing else — it is deliberately a separate commit rather than folded into the others, so that a review already in progress stays valid.
Compatibility notes
Everything Squidex touches was verified present and unchanged across 4.8 → 4.15:
ObjectInstance.Extensibleis stillpublic virtual;GetOwnProperty,Set,DefineOwnProperty,RemoveOwnProperty,GetOwnProperties,GetOwnPropertyKeyssignatures unchanged;PropertyDescriptor(PropertyFlag)andprotected virtual CustomValueunchanged, soCustomPropertyandContentFieldPropertybehave as before;Engine.Constraints.Reset(),Options.Constraints.PromiseTimeout,AllowClrWrite,EvaluateAsync(in Prepared<Script>, CancellationToken),CancellationToken(ct),ObjectWrapper.Create,JsDate(engine, DateTime),JsValue.FromObjectall present; theJsArray/JsStringpattern matches inJintExtensions.ToIdsandScriptOperations.Rejectare unaffected.Transitive dependency: Acornima moves to 1.6.2.
ParseErrorException,ScriptPreparationExceptionandJintException, whichJintScriptEngine.MapExceptionswitches on, all still exist.Constraints need no change.
TimeoutIntervalandCancellationTokenare the kind that keeps Jint's interpreter fast path armed.Package provenance. After restore,
.nupkg.metadatareportshttps://api.nuget.org/v3/index.jsonand the nuspec repository commit isa304aa5dacd340e2a5ff51e1ea0c465e38e50aa8, the v4.15.3 tag.Also unchanged and already right:
Prepared<Script>is cached viaCacheParserand shared across engines (Jint documents that as safe), andStrict()is on.The branch is still named 4.15.2 because renaming it would close this PR; the pinned version is 4.15.3.