Repository navigation
perf(runtime): reuse wrappers before resolving runtime classes - #40
Merged
Merged
Conversation
Interface-typed and Object returns looked up the object's runtime class (GetRuntimeClassName, a metadata lookup) before checking the wrapper cache, so returning an already-wrapped object paid for both on every call, twice for interface-typed returns. The cache is now checked first on both engines; napi records which cached wrappers were built for the object's class so an interface-typed return can reuse them. Runtime class names convert from UTF-16 directly instead of formatting one character at a time, which was about half of the lookup's cost. JsonArray.GetAt on cached elements: 2.0M -> 4.8M calls/s (classic V8).
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Interface-typed and
Objectreturns looked up the object's runtime class (GetRuntimeClassNameplus a metadata lookup) before checking the wrapper cache. The lookup came in with #37, so since then, returning an object that already had a JS wrapper paid for it on every call, and interface-typed returns paid twice. In aJsonArray.GetAtloop the lookup was about 25% of the time, and about half of that wasHSTRING::to_string, which goes throughcore::fmtone character at a time.Changes (classic + napi)
return_value_from_kindand thens_proxybuilders) checkwrapper_cachebefore resolving the class. New helpers:wrapper_cache::get_fortakes anIUnknown, andtake_cachedtakes an owned return pointer and releases it on a hit.is_class), so an interface-typed return reuses it by COM identity without asking for the runtime class.HSTRING::to_string_lossy(a direct UTF-16 decode).GetRuntimeClassNamecall inmethod_call.rswhose result was discarded.Benchmarks
Classic V8 playground, medians of alternating A/B runs against
main:JsonArray.GetAt(cached element)JsonObject.GetNamedValueset_get_named, constructTesting
cargo test -p runtime --lib -- projection_test instance_cache wrapper: 11/11.classic,napi_engine).