[EXPERIMENT] Send render request body as JSON (closes #3280 hypothesis)

[AbanoubGhadban]

Experiment for #3280. Not for merging as-is — no tests, no version negotiation, no docs, no migration story. Per the issue's scope: implement in the simplest possible way and measure.

Change

One-line conditional in react_on_rails_pro/lib/react_on_rails_pro/request.rb ([+9, -1]):

def render_code(path, js_code, send_bundle)
  Rails.logger.info { "[ReactOnRailsPro] Perform rendering request #{path}" }
  form = form_with_code(js_code, send_bundle)
  if send_bundle
    perform_request(path, form: form)    # keep multipart for bundle upload
  else
    perform_request(path, json: form)    # JSON for steady-state render
  end
end

send_bundle: true keeps the multipart path because form_with_code embeds the bundle as a Pathname which JSON.generate can't serialize. Streaming render (render_code_as_stream) and incremental render (render_code_with_incremental_updates) are intentionally untouched per the issue's scope. No Node-side change is needed — Fastify's built-in JSON parser already handles application/json for the /bundles/.../render/... endpoint.

Headline results

Workload: /mega_benchmark_traditional?u=500&p=1000&c=5000 on rsc-benchmar's apps/ror-rsc (props ~1.16 MB, HTML ~2.18 MB). Branch base: upcoming-v16.3.0 (3d571c32).

Metric	Acceptance bar	Baseline	After	Δ	Passes?
Median total wall	drop ≥ 150 ms	396 ms	92 ms	−304 ms	✅ +154 ms
Median GC per request	drop ≥ 15 ms	112.8 ms	9.8 ms	−103 ms	✅ +88 ms
Byte-equivalent HTML	identical	—	—	UUID-only drift	✅
Streaming SSR no regression	flat or better	—	—	±noise	✅

Sequential sweep (n=25 per point, 110-call warmup)

Improvement scales with payload size, exactly as the encoding-cost hypothesis predicts:

Point (u,p,c)	base total / gc	after total / gc	Δ total
0, 0, 0	7 / 0.1	7 / 0.0	0
500, 0, 0	45 / 4.3	34 / 0.6	−11
500, 1000, 0	185 / 31.5	41 / 1.5	−144
500, 1000, 2500	238 / 57.4	55 / 4.2	−183
500, 1000, 5000	396 / 112.8	92 / 9.8	−304

Isolation (n=30 per endpoint, u=500&p=1000&c=5000)

build_only (controller alloc, no renderer call) is unchanged — confirms the savings live in the renderer round-trip path the change touches, not in generic system warmup:

Endpoint	base total / gc	after total / gc	Δ total
build_only (controller alloc only)	34 / 2.8	32 / 2.9	−2
send_only (renderer + view, memoized props)	306 / 81.5	86 / 6.1	−220
full (both)	339 / 95.9	97 / 6.9	−242

Concurrent load (autocannon --sweep --duration=20)

Traditional SSR pages (the change applies):

Page	c	base p50	after p50	Δ p50	base rps	after rps
hello_world	1 / 10 / 50	10 / 42 / 224	10 / 40 / 231	flat	193 / 206 / 198	224 / 214 / 202
heavy_benchmark_traditional	10	176	102	−74	54	94
heavy_benchmark_traditional	50	950	588	−362	51	82
mega_benchmark_traditional	1	308	101	−207	3	10
mega_benchmark_traditional	10	2625	955	−1670	3	10
mega_benchmark_traditional	50	8480 / 69 err	4668 / 0 err	−3812	2	10

Streaming SSR (hello_server, mega_benchmark) is flat ±noise — they go through render_code_as_stream, which we left untouched, and the bench confirms no regression.

Robustness — three independent measurement methodologies agree

To rule out time-varying environmental noise (thermal throttle, OS background activity, CPU governor drift), I also ran two complete Rails+renderer stacks side-by-side simultaneously on the same machine: exp on :3000/:3800 (with the change), baseline on :3002/:3810 (without).

Method	mega p50 Δ
Sequential (consecutive runs, no parallel stack)	−304 ms
Paired-curl (alternating requests, both stacks live, no load)	−254 ms
Parallel autocannon c=1 (both stacks under load)	−272 ms

All three within ~50 ms of each other. The slightly smaller delta on the simultaneous methods is symmetric CPU contention from two stacks sharing a 12-core box; the comparison stays fair because both sides see the same interference.

Robustness — does PR #3217 (TCP_NODELAY) eat the win?

Also tested with the #3217 TCP_NODELAY monkey-patch applied to baseline only (verified via HTTPX::TCP.ancestors.include?(ReactOnRailsPro::HttpxTcpNodelayPatch) == true). Re-ran paired-curl + parallel autocannon:

Workload	Δ without #3217 on baseline	Δ with #3217 on baseline
mega paired-curl	+254 ms	+301 ms
mega autocannon c=1	+272 ms	+279 ms
mega autocannon c=10	+2722 ms	+2747 ms
mega autocannon c=50	+1696 ms	+2436 ms

The win is unchanged. #3217 targets Nagle + delayed-ACK on small writes — but the renderer round-trip is large-burst (1.16 MB props, 2.18 MB HTML), so Nagle never engages, and on localhost loopback (min-RTT ~7 µs) the 40 ms tail doesn't form anyway. The two PRs target orthogonal bottlenecks: #3217 helps network-RTT-bound scenarios; #3280 helps payload-encoding-bound scenarios.

Correctness

Captured HTML before and after for /hello_world, /heavy_benchmark_traditional, /mega_benchmark_traditional?u=500&p=1000&c=5000. All three byte-equal: 7,690 / 59,713 / 2,186,638 bytes. diff shows only the per-request random UUID drift on HelloWorld-react-component-<uuid> DOM ids — same drift you see between two consecutive baseline calls.

Setup

react_on_rails branch	experiment/3280-json-render-body off upcoming-v16.3.0 (3d571c32)
Single commit	352743ef1 — 9 added, 1 modified lines in request.rb
Bench harness	AbanoubGhadban/rsc-benchmar branch fair-comparison-fixes, apps/ror-rsc
Ruby / Node / Rails / Puma	3.3.0 (YJIT) / 22.12.0 / 7.2.3.1 / 8.0.1
OS / CPU	Linux 6.8, Intel i7-9750H (12 logical cores)
Renderer	3 workers, http_pool_size=64, pool_timeout=30s
DB	Postgres, 500 users / 1000 posts / 5000 comments
jemalloc	not preloaded

Recommendation

Verdict: Go. Open a production-quality follow-up issue that addresses everything this experiment intentionally skipped:

1. Backward compatibility with older node renderers. The gem speaks JSON only against renderers that have Fastify's default JSON parser enabled (all current versions do, but worth confirming the floor). A content-type negotiation or version handshake may be needed if any users are on a custom-configured Fastify that disabled the default JSON parser.

2. Bundle-warmup path stays multipart — already handled by the send_bundle conditional, but worth adding a test that exercises the warmup path so a future refactor doesn't break it.

3. Apply the same flip to render_code_with_incremental_updates? It already uses application/x-ndjson, but the initial line is JSON-in-multipart-envelope — same opportunity in principle. Out of scope here; needs separate measurement.

4. Tests covering the conditional form: / json: choice, including a regression test that confirms byte-equivalent HTML output across both encodings.

5. Docs / changelog entry calling out the wire-protocol change and the perf characteristics.

6. Defensive audit of form_with_code — confirm there's no future code path that could put a non-JSON-serializable value into the hash when send_bundle: false.

#3217 (TCP_NODELAY) and this PR are orthogonal optimizations. Merging both gives the cumulative benefit of each on whichever workload it targets — payload-bound (mega-page) gets the JSON win, RTT-bound (smaller frequent writes over real network) gets the TCP_NODELAY win.

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0b503387-85be-443f-b948-935f8ece8396

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch experiment/3280-json-render-body

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[claude]

Code Review

Overview

This experiment PR swaps the render_code method's wire protocol from multipart/form-encoded to application/json for steady-state renders (when send_bundle: false), while keeping the multipart path for bundle-upload renders. The benchmark numbers are compelling — up to −304 ms median wall time and −103 ms GC on large payloads — and the methodology is rigorous (three independent measurement approaches, side-by-side concurrent stacks, isolation controls).

The change is tiny and correct in its current form, but the items below should be resolved before a production PR is opened.

---

Issues

1. No tests for render_code (critical gap)

The existing request_spec.rb has good coverage of render_code_as_stream and render_code_with_incremental_updates, but zero tests for render_code itself. The new conditional changes the content-type and serialization format but there's nothing to verify:

• send_bundle: false → application/json with correct JSON body
• send_bundle: true → multipart path is preserved
• JSON body content matches what the node renderer expects

The tests at lines 155/165 of request_spec.rb check that render_code_as_stream uses form encoding (renderingRequest=console.log). After this change, render_code would produce {"renderingRequest":"console.log..."} — the absence of a parallel assertion for the JSON path is a coverage hole.

2. nil serialization difference for dependencyBundleTimestamps

Utils.common_form_data sets "dependencyBundleTimestamps" => dependencies where dependencies is nil when RSC support is disabled (line 168–171 of utils.rb).

• Form encoding: nil values are typically omitted or sent as empty string
• JSON: JSON.generate serializes nil as null, producing {"dependencyBundleTimestamps":null,...}

The node renderer's JSON parser needs to handle null for this field correctly. This is likely fine given Fastify's built-in parser, but it's a subtle behavior difference from the form-encoded path and should be explicitly verified and tested.

3. Comment style violates project conventions

The added comment is 4 lines including "Experiment" language. Project conventions (CLAUDE.md) specify: "Default to writing no comments. Only add one when the WHY is non-obvious — one short line max. Never write multi-line comment blocks."

If the comment is kept at all in a production PR, it should be reduced to a single line that captures the invariant (e.g., # Keep multipart when send_bundle=true — bundle Pathname can't be JSON-serialized).

4. No backward-compatibility guard for older node renderers

The PR description acknowledges this, but it's worth flagging for the follow-up issue: there is nothing in the code that enforces a minimum node renderer version. If a user upgrades the gem but not the renderer, and the renderer has a non-default Fastify configuration that doesn't accept application/json on the render endpoint, the failure will be a cryptic 4xx/5xx rather than a clear compatibility error. A version check in the follow-up PR is important.

5. Intentional asymmetry between render_code and render_code_as_stream

render_code_as_stream still uses form encoding (perform_request(path, form: form, stream: true) at line 54). The PR description explicitly calls this out as out-of-scope, which is fine — but the follow-up issue should track applying the same flip there (and to render_code_with_incremental_updates) with separate benchmark evidence.

---

Minor nit

perform_request already has form: nil, json: nil defaults and raises ArgumentError if neither is provided, so the new conditional is correctly wired to the existing infrastructure. No concerns there.

---

Verdict

The hypothesis is proven and the experiment is well-executed. The change itself is mechanically sound. Before a production PR:

1. Add unit tests for both branches of the new conditional
2. Verify null vs omitted for dependencyBundleTimestamps on the Node side
3. Trim the comment to project conventions
4. Open a tracking issue for version negotiation + streaming path parity

[claude]

Watch out for a subtle nil vs omitted difference here. Utils.common_form_data includes "dependencyBundleTimestamps" => dependencies where dependencies is nil when RSC support is disabled. With form encoding that value is typically omitted or sent as an empty string; JSON.generate will produce "dependencyBundleTimestamps":null. The node renderer needs to explicitly handle null for that field — worth a unit test and a Node-side check before the production PR.

[claude]

4-line block comment and "Experiment" language are fine for an experiment branch, but project conventions (CLAUDE.md) cap comments at one line and restrict them to non-obvious WHY. For the production PR this should be reduced to something like:

Suggested change

	# Experiment (issue #3280): use JSON for steady-state render to skip
	# form-urlencode CPU/GC. Keep multipart when send_bundle=true because
	# form_with_code embeds the bundle file as a Pathname which can't be
	# JSON-serialized.
	# Keep multipart when send_bundle=true — bundle Pathname can't be JSON-serialized.

[greptile-apps]

Greptile Summary

This experiment PR replaces the multipart/form-data encoding with application/json for steady-state (no-bundle) render requests in render_code, confirming the encoding-cost hypothesis from issue #3280 with a median wall-time drop of ~304 ms on large payloads.

• The one-line conditional correctly guards the multipart path when send_bundle: true (Pathname objects can't be JSON-serialized), and correctly uses the existing encode_request_body(json:) path for the steady-state case.
• render_code_as_stream is intentionally left on form: encoding even though it always passes send_bundle: false; a production follow-up should flip it too.
• There is no version negotiation with the Node renderer; Fastify's built-in JSON parser covers current versions, but a guard will be needed before this lands in a release.

Confidence Score: 4/5

Safe to keep as a benchmark branch; not safe to merge into a release without adding renderer version negotiation, tests, and applying the same JSON flip to the streaming path.

The change is a single conditional in one method, backed by thorough benchmark data and correct use of the already-exercised JSON encoding path. The streaming counterpart (render_code_as_stream) still sends multipart, creating asymmetric behavior. No renderer capability check exists, so a renderer that does not support application/json on the render endpoint would break silently. Both gaps are acknowledged in the PR description as intentional experiment scope.

react_on_rails_pro/lib/react_on_rails_pro/request.rb — specifically the render_code_as_stream method and the absence of a content-type/version guard around the new JSON branch.

Important Files Changed

Filename	Overview
react_on_rails_pro/lib/react_on_rails_pro/request.rb	Adds a conditional in render_code to send application/json instead of multipart form when send_bundle is false; relies on encode_request_body's existing JSON path; render_code_as_stream stays on form encoding even though it always passes send_bundle=false

Comments Outside Diff (1)

1. react_on_rails_pro/lib/react_on_rails_pro/request.rb, line 38-55 (link)

   render_code_as_stream misses the same JSON opportunity

   render_code_as_stream always calls form_with_code(js_code, false) (hard-coded false) and then sends via form:, so it keeps multipart encoding even though the bundle is never included in this path. The non-streaming and streaming steady-state paths now diverge in wire format. If this optimization is promoted to production, streaming renders will still pay the form-encoding CPU/GC cost that motivated the change, even though there is no serialization obstacle (send_bundle is never true here). The PR description flags this as intentionally out of scope, but it is worth capturing so the follow-up issue includes it.

_{Reviews (1): Last reviewed commit: "[EXPERIMENT] Send render request body as..." | Re-trigger Greptile}

[greptile-apps]

No renderer-side version guard before switching content type

Switching from multipart/form-data to application/json is a wire-protocol change. Any Node renderer that has disabled Fastify's default JSON body parser (e.g. a custom plugin stack, an older version, or a user-patched renderer) will start responding with 415 Unsupported Media Type or silently misparse the body, and the gem has no way to detect or fall back. The existing STATUS_INCOMPATIBLE check only fires when the renderer explicitly returns that status code; it won't catch a JSON-parse failure on the renderer side. The PR description calls this out for the follow-up, but it is worth documenting in a TODO comment near the branch so the production PR doesn't inadvertently ship without it.

[AbanoubGhadban]

Closing — this was the intentionally-throwaway experiment for #3280 (no tests/version-negotiation/docs, per the issue's scope). The full experiment is now preserved in the planning doc, including the verbatim diff, all benchmark numbers, the operational problems hit, and a phased production plan:

→ #3299 — internal/planning/json-render-body-migration-plan.md

Production work will be a fresh PR per §5 of that doc (protocol-version-gated, with tests + docs). The orthogonal #3217 (TCP_NODELAY) is unaffected and left open.