GeezerVison

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

GeezerVison is showing github.com — perf: ArrayBuffer.from without the isDirect() call by triniwiz · Pull Request #2076 · 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

ArrayBuffer.from(byteBuffer) called ByteBuffer.isDirect() in Java to choose between the direct and copying paths. GetDirectBufferAddress already returns null for a heap buffer, so the null check now picks the path and the Java call is gone. A direct buffer whose address is unavailable falls back to the copying path, which works for both kinds.

The nativeObject key set on the returned buffer is now one internalized v8::Eternal string, so a new string is no longer made on every call.

Measurements

Measured together with #2075 (same setup and numbers there): grid-dashboard and tree-fanout attached 0.96, other scenarios flat. This part on its own removes one JNI call into Java and one string allocation per ArrayBuffer.from.

Testing

:runtime:assembleRelease -Poptimized -Pabis=arm64-v8a builds; ran in the benchmark app as above.
The runtime test suite has not been run.

Summary by CodeRabbit

Bug Fixes
- Improved handling of Java byte buffers when creating array buffers. Direct buffers can share their underlying storage, while other buffers retain their remaining bytes through a copy. This helps ensure buffer contents are available in the resulting array buffer across both buffer types.

Sorry, something went wrong.

Uh oh!

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

All reactions


          perf: ArrayBuffer.from without the isDirect() call

17268bf

GetDirectBufferAddress already returns null for a heap buffer, so the
Java isDirect() call is not needed to pick the path. The "nativeObject"
key is now one internalized string instead of a new string per call.

triniwiz mentioned this pull request

Oct 31, 2026

perf: skip Class.getName() when wrapping a returned object of a seen class #2075

Open

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: cd630ee9-65de-43f4-b8b6-b6d796337fa8

Commits

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

Files selected for processing: 2

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

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

Walkthrough

ArrayBufferHelper now checks the direct buffer address to select a direct backing store or copy the remaining bytes. It also caches the "nativeObject" V8 key used to attach the Java object to the returned array buffer.

Changes: ArrayBuffer conversion

Layer / File(s)	Summary
Backing-store selection and object attachment `test-app/runtime/src/main/cpp/ArrayBufferHelper.cpp`, `test-app/runtime/src/main/cpp/ArrayBufferHelper.h`	The helper checks `GetDirectBufferAddress()` to select a direct backing store or copy the remaining bytes into a new backing store. It replaces the JNI method ID with a cached V8 key for `"nativeObject"` and uses that key when attaching the Java object.

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

Change: Refactor

Suggested reviewers: edusperoni

Merge Risk: Minimal · up to 17268

No actionable merge-blocking risk is established; the change is mergeable with normal checks.

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: ArrayBuffer.from no longer calls Java ByteBuffer.isDirect(). The raw summary confirms ArrayBufferHelper stops calling isDirect() and uses GetDirectBufferAddress() with…
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: ArrayBuffer.from no longer calls Java ByteBuffer.isDirect(). The raw summary confirms ArrayBufferHelper stops calling isDirect() and uses GetDirectBufferAddress() with a null check. The title also has the conventional 'perf:' prefix. It omits the secondary nativeObject key change, which is acceptable because the title does not need every detail.

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 2 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 buffer’s way,
A direct address leads the day.
If none is found, the bytes are copied,
A cached key keeps the object adopted.
Then off the bunny bounds away.}

All reactions

Sorry, something went wrong.

Uh oh!

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

triniwiz marked this pull request as ready for review

November 2, 2026 22:08

edusperoni approved these changes

Nov 5, 2026

View reviewed changes

edusperoni merged commit 1a402f0 into main

Nov 5, 2026

8 checks passed

Uh oh!

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

edusperoni deleted the perf/arraybuffer-from branch

November 5, 2026 19:53

ammarahm-ed mentioned this pull request

Nov 9, 2026

perf(android): create ArrayBuffer.from without calling ByteBuffer.isDirect NativeScript/runtimes#132

Open

Reviewers

edusperoni

edusperoni approved these changes

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.

2 participants

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!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

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.