GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — perf: skip Class.getName() when wrapping a returned object of a seen class by triniwiz · Pull Request #2075 · NativeScript/android

Conversation

triniwiz commented Oct 31, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

What

When a Java method returns an object that has no JS wrapper yet, ObjectManager::CreateJSWrapper called Class.getName() in Java, copied the jstring into a std::string, replaced . with /, and then looked the class up again with FindClass for Link.

ObjectManager now remembers up to four runtime classes per declared type. Each is matched with IsSameObject, which runs no Java code, and the cached class name and global class are reused. On a miss the old path runs, with FindClass still after the wrapper is created. The cached jclass values come from JEnv's class cache, whose global refs are never deleted.

Measurements

Pixel 9 Pro emulator (arm64, headless), xplat-benchmarks NativeScript app, size M, release build, 5 interleaved rounds. Both runtimes were built from main with and without this change and #2076 (measured together). attached B/A:

Scenario	attached	per round
grid-dashboard	0.96	0.95 0.95 0.99 0.97 0.98
tree-fanout	0.96	1.02 0.93 0.92 0.96 1.00
flex-wrap-tiles	1.00
styled-cards	1.01
text-flow	1.00

The gain is small; most of the per-object cost is still getOrCreateJavaObjectID, which this does not touch.

Testing

:runtime:assembleRelease -Poptimized -Pabis=arm64-v8a builds; the runtime swapped into a 9.1.1 app ran every benchmark round.
The runtime test suite has not been run.

Summary by CodeRabbit

Performance
- Java-backed object wrappers can now be created with less repeated class lookup work, which may improve wrapper creation efficiency.
- Wrapper creation continues to use the runtime class when it differs from a previously linked class.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          perf: skip Class.getName() when wrapping a returned object of a seen …

f9c8af5

…class

Wrapping a Java object returned to JS called Class.getName() in Java,
copied the jstring, and looked the class up again with FindClass for Link.
ObjectManager now keeps up to four runtime classes per declared type,
matched with IsSameObject (no Java code runs), and reuses the class name
and global class. On a miss FindClass still runs after the wrapper is
created, as before.

coderabbitai Bot commented Oct 31, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

No actionable comments were generated in the recent review.

Recent review info

Run configuration

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: f236a886-20aa-4352-89f9-d82d629b4fd5

Commits

Reviewing files that changed from the base of the PR and between bdda733 and f9c8af5.

Files selected for processing: 2

test-app/runtime/src/main/cpp/ObjectManager.cpp
test-app/runtime/src/main/cpp/ObjectManager.h

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Walkthrough

ObjectManager now keeps up to four linked Java classes per declared type. When creating a wrapper for an instance, it reuses a cached class if it matches the instance’s class. Otherwise, it resolves the runtime class name, creates the wrapper, and caches the linked class when linking succeeds.

Changes: Wrapper class cache

Layer / File(s)	Summary
Linked class contract and helper `test-app/runtime/src/main/cpp/ObjectManager.h`, `test-app/runtime/src/main/cpp/ObjectManager.cpp`	`CreateJSWrapperHelper` accepts a class name, an optional class reference, and an optional output pointer for the linked class. It calls `FindClass` only when no class reference is supplied.
Instance wrapper cache flow `test-app/runtime/src/main/cpp/ObjectManager.h`, `test-app/runtime/src/main/cpp/ObjectManager.cpp`	`ObjectManager` stores recent class entries by declared type. It reuses an entry when its class is JNI-identical to the instance’s class and removes the oldest entry when the cache already has four entries.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Suggested reviewers: edusperoni

Merge Risk: Minimal · up to f9c8a

No material merge-blocking issue was established. The runtime test suite has not been run.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title names the main change: wrapping a returned object of an already-seen class skips the Class.getName() call. The raw summary and PR objectives both describe a per-class cache in ObjectManager:…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title names the main change: wrapping a returned object of an already-seen class skips the Class.getName() call. The raw summary and PR objectives both describe a per-class cache in ObjectManager::CreateJSWrapper that reuses the cached class name and class reference on a hit. The title's 'perf:' prefix matches the performance intent stated in the objectives. The title is specific and on topic.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 4 functions across 2 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

Generate docstrings

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

_{A rabbit checks the class cache bright,
And finds a match by JNI sight.
Four linked names are kept in store,
The oldest hops out at the door.
New wrappers spring to life with care.}

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

triniwiz mentioned this pull request

Oct 31, 2026

perf: ArrayBuffer.from without the isDirect() call #2076

Merged

triniwiz marked this pull request as ready for review

November 2, 2026 22:08

edusperoni mentioned this pull request

Nov 9, 2026

perf: faster object lifecycle and JNI calls #2099

Merged

This branch has not been deployed

No deployments

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

1 participant

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.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

triniwiz commented Oct 31, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Measurements

Testing

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

triniwiz commented Oct 31, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

coderabbitai Bot commented Oct 31, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.