Skip to content

Optimize reflection binding for slices and maps#168

Open
repyh wants to merge 2 commits into
mainfrom
daily-optimization-bridge-core-13976936210534244333
Open

Optimize reflection binding for slices and maps#168
repyh wants to merge 2 commits into
mainfrom
daily-optimization-bridge-core-13976936210534244333

Conversation

@repyh
Copy link
Copy Markdown
Owner

@repyh repyh commented May 29, 2026

Optimized bindSlice and bindMap to improve reflection performance.


PR created automatically by Jules for task 13976936210534244333 started by @repyh

Summary by CodeRabbit

  • Bug Fixes

    • Optimized Go→JavaScript type conversions for better runtime performance.
    • Enhanced byte-slice handling with native Uint8Array/ArrayBuffer support when available.
    • Improved map iteration and key-to-string conversion efficiency.
  • Chores

    • Reduced unnecessary imports in build/run outputs by skipping uninspected module imports.

Review Change Stack

Added a fast-path in `bindSlice` for `[]byte` by wrapping it in `Uint8Array` after copying the data.
Optimized `bindMap` by utilizing `MapRange()` and `strconv` to prevent slice allocation and interface boxing.
Added `strconv` to the imports of `bridge/core/reflection.go`.

Co-authored-by: repyh <63894915+repyh@users.noreply.github.com>
@google-labs-jules
Copy link
Copy Markdown
Contributor

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 29, 2026

📝 Walkthrough

Walkthrough

Adds Sobek runtime optimizations: fast-path for []byte -> ArrayBuffer/Uint8Array and improved map-key stringification using strconv. Separately, the CLI tracks inspected Go modules and omits uninspected dotted imports when generating Go shim import blocks.

Changes

Sobek Type Binding Optimizations

Layer / File(s) Summary
Byte slice to Uint8Array fast path
bridge/core/reflection.go
bindSlice detects []byte, clones bytes into an ArrayBuffer, and attempts to construct a Uint8Array; falls back to ArrayBuffer if typed-array construction fails.
Map binding with optimized key conversion
bridge/core/reflection.go
bindMap uses MapRange() and refines key stringification: string keys used directly, integer/unsigned keys formatted via strconv, and other kinds fall back to fmt.Sprint.

CLI inspected-modules tracking

Layer / File(s) Summary
Record inspected modules during inspection
pkg/cli/cmd/build.go, pkg/cli/cmd/run.go
Introduces inspectedModules maps and marks cleanImp entries after successful linker.Inspect calls to record which modules were inspected.
Skip uninspected dotted imports in importBlock
pkg/cli/cmd/run.go, pkg/cli/cmd/build.go
When generating the final Go shim importBlock, skip adding imports whose cleanImp was not inspected and contains a dot (.).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 In bridges where Go and JS entwine,
bytes become buffers, aligned just fine,
maps iterate with keys made neat,
inspected modules skip imports obsolete —
hop, review, and ship the fine design!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main changes: optimization of reflection binding for slices and maps in bridge/core/reflection.go, which is the primary focus per the commit messages and raw summary.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch daily-optimization-bridge-core-13976936210534244333

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

Comment @coderabbitai help to get the list of available commands and usage tips.

Added a fast-path in `bindSlice` for `[]byte` by wrapping it in `Uint8Array` after copying the data.
Optimized `bindMap` by utilizing `MapRange()` and `strconv` to prevent slice allocation and interface boxing.
Added `strconv` to the imports of `bridge/core/reflection.go`.
Updated `pkg/cli/cmd/build.go` and `pkg/cli/cmd/run.go` to skip uninspected modules.

Co-authored-by: repyh <63894915+repyh@users.noreply.github.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
pkg/cli/cmd/run.go (1)

89-132: ⚡ Quick win

Consider extracting the inspectedModules tracking pattern into a shared helper.

The logic for tracking inspected modules (initialize map → mark after successful linker.Inspect → skip uninspected dotted imports) is duplicated between build.go and run.go. Additionally, there's a subtle inconsistency: run.go line 100 checks fetcher.Get errors before calling Inspect, while build.go line 67 ignores them.

Extracting this pattern would ensure consistent behavior and reduce maintenance overhead.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/cli/cmd/run.go` around lines 89 - 132, Extract the duplicated "inspect
and record go: imports" logic into a single helper (e.g.,
InspectAndRegisterModules) that accepts the imports list, fetcher, linker and
references to bindBlock and virtualModules and returns the inspectedModules map;
have both run.go and build.go call it so the logic for calling fetcher.Get,
calling linker.Inspect, setting inspectedModules[cleanImp]=true, appending
linker.GenerateShim to bindBlock and populating virtualModules is centralized
and consistent (decide and implement a single policy for handling fetcher.Get
errors vs ignoring them and apply it inside the helper) so the check later that
skips uninspected dotted imports uses the same inspectedModules result.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@pkg/cli/cmd/run.go`:
- Around line 89-132: Extract the duplicated "inspect and record go: imports"
logic into a single helper (e.g., InspectAndRegisterModules) that accepts the
imports list, fetcher, linker and references to bindBlock and virtualModules and
returns the inspectedModules map; have both run.go and build.go call it so the
logic for calling fetcher.Get, calling linker.Inspect, setting
inspectedModules[cleanImp]=true, appending linker.GenerateShim to bindBlock and
populating virtualModules is centralized and consistent (decide and implement a
single policy for handling fetcher.Get errors vs ignoring them and apply it
inside the helper) so the check later that skips uninspected dotted imports uses
the same inspectedModules result.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0063f7bc-0050-4465-b983-a98d7b23a7df

📥 Commits

Reviewing files that changed from the base of the PR and between bbdc677 and 1bcc6c1.

📒 Files selected for processing (2)
  • pkg/cli/cmd/build.go
  • pkg/cli/cmd/run.go

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant