Skip to content

perf: decode_bytearray (closes #570) + hot-path smart_decode shortcuts (credit @markjm #575) - #5

Merged
Tagar merged 2 commits into
masterfrom
perf-quick-wins
May 21, 2026
Merged

Tagar merged 2 commits into
masterfrom
perf-quick-wins

Conversation

@Tagar

@Tagar Tagar commented May 21, 2026 •

Copy link
Copy Markdown
Member

Two complementary perf wins on the recv path, plus tightened protocol-layer code. The bytes-decode optimization (issue py4j#570) lands alongside the hot-path smart_decode shortcuts from @markjm's py4j#575 — carefully, with the bytes/str regression from py4j#575 fixed.

Commit 1 — decode_bytearray (closes py4j#570, credit @PaperTsar)

Drops the byte-list intermediate that py2-compat required. ~7.5× microbench speedup on 256 KB payloads (200 ms → 32 ms, 100 iterations).

# Before:
return bytes([b for b in standard_b64decode(new_bytes)])  # one PyObject per byte

# After:
return bytes(standard_b64decode(encoded.encode("ascii")))

Return type stays bytes (matches the existing bytearray2 = bytes contract; downstream consumers rely on this).

Commit 2 — hot-path smart_decode shortcuts (credit @markjm py4j#575)

smart_decode(bytes) dispatches through an isinstance check before calling .decode("utf-8"). At every per-call socket recv site, the input is known bytes (stream opened via socket.makefile("rb")), so the dispatch is pure overhead. This commit decodes at the source.

Sites changed:

  • GatewayConnection.send_command (every JavaGateway round-trip)
  • do_client_auth + the callback-server connection's run / _call_proxy / _get_params (4 sites)
  • OutputConsumer.run (stdout consumer)
  • 7 sites in clientserver.py
  • launch_gateway auth-token decode (without this, auth comparison silently fails: bytes != str)

Plus minor: encode_float uses str(float_value) directly (same shortest-roundtrip repr on py3); smart_decode(addr/port/id) → str(...) for finalizer keys / proxy IDs / logging.

What's deliberately NOT adopted from py4j#575

  • smart_decode in escape_new_line is KEPT. Dropping it makes bytes.replace("str", "str") raise TypeError — this was the testGatewayAuth regression I flagged on the upstream PR (see comment 4484469752). The replace chain is fast enough that the smart_decode dispatch isn't a hot-path concern, and the safety net is load-bearing for any caller that hands escape_new_line bytes. Docstring updated with the rationale.

  • decode_bytearray return type stays bytes, not bytearray. perf: Remove Python 2 compat and optimize old calls to smart_decode py4j/py4j#575 changed it; this PR keeps the existing contract.

Tests

  • EscapeNewLineBytesInputSafetyTest (new, 4 tests) — pins the bytes-input contract on escape_new_line (ASCII bytes / str / UTF-8 bytes / empty bytes). If a future refactor drops the smart_decode safety net, these fail immediately — before the JVM-bound testGatewayAuth would. This is the regression-guard I was missing in my upstream review.

  • GatewayLauncherTest.testGatewayAuth (existing) — full launch_gateway(enable_auth=True) end-to-end. Validates the bytes-stream auth-token read path.

  • PythonEntryPointTest.test_python_entry_point_with_auth (existing) — callback-with-auth through all 4 modified read sites in the callback server.

CodSpeed expectation

Credit:

Co-authored-by: Isaac

@codspeed

codspeed Bot commented May 21, 2026 •

Copy link
Copy Markdown

Merging this PR will improve performance by 91.12%

⚡ 2 improved benchmarks
✅ 16 untouched benchmarks
⏩ 7 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
⚡ WallTime test_m5c_encode_float 3 µs 2.7 µs +10.17%
⚡ WallTime test_macro_scenario[X7-16k] 102.9 ms 31 ms ×3.3

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.


Comparing perf-quick-wins (e7b1485) with master (3c2f164)

Open in CodSpeed

Footnotes

  1. 7 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@Tagar
Tagar force-pushed the perf-quick-wins branch from 154afe2 to 6651d5e Compare May 21, 2026 07:08
@Tagar Tagar changed the title Perf: low-risk hot-path wins (clientserver recv, protocol constants, decode_bytearray) perf: decode_bytearray (~7.5x on bytes recv path) + X7 macro baseline May 21, 2026
Tagar added a commit that referenced this pull request May 21, 2026
Complements the existing perf framework (M-micro / X-macro scenarios)
and the per-PR CodSpeed tracking with measurement-reporting tests
that print throughput / latency numbers and catch dramatic
regressions.

What's added:

* **X8 (perf framework): bytes SEND macro scenario** at 1k / 16k /
  256k. Complements X7 (bytes recv) by exercising the encode_bytearray
  + Python->Java send path. Same BAOS.write(byte[]) Nagle-sensitive
  surface. X8-16k joins the CodSpeed CI scenario list.

* **ColdStartLatencyTest** (issue py4j#557) — measures
  `JavaGateway() + first-call` latency once. JVM init is excluded
  (handled by start_example_app_process); the timed window is what
  py4j actually controls. Prints the value; asserts <2s.

* **First-vs-warm ratio test** (issue py4j#557) — same gateway, first
  call vs median of 100 warm calls. Asserts <100x ratio (loose
  enough to not flake; tight enough to catch a real cold-path
  regression).

* **StringArgLatencyCurveTest** — per-call latency at string args
  of 16 / 256 / 4K / 16K / 64K bytes. Fills the gap between M2b
  (5-byte arg) and X7-class large-payload scenarios; surfaces the
  Nagle-knee around the BufferedWriter buffer boundary on Linux.

* **BandwidthSummaryTest** — MB/s at 1K / 16K / 256K for both
  recv (Java->Python ByteBuffer.array()) and send (Python->Java
  byte[] argument). Locally on macOS loopback: recv 7→94 MB/s,
  send 6→245 MB/s — confirms send is faster (no base64 decode)
  and predicts the decode_bytearray gain from #5.

Validation handle for the open PRs:
* PR #5 (decode_bytearray) — `BandwidthSummaryTest` recv numbers
  should improve substantially after the list-comprehension fix.
* PR #6 (Nagle) — `StringArgLatencyCurveTest` should show a
  flatter curve across the BufferedWriter boundary; `X8-16k` /
  `X7-16k` (#5) drop dramatically on Linux runners.

Tests print results when run with `pytest -v -s`; without `-s`
pytest captures the prints. Asserts use generous bounds to catch
catastrophic regressions without being CI-flaky on hardware-
dependent absolute numbers.

Co-authored-by: Isaac
Tagar added a commit that referenced this pull request May 21, 2026
This PR establishes a measurement baseline that the open optimization
PRs (decode_bytearray #5, Nagle #6) can be validated against, plus
adds regression-guard coverage for several blind spots.

Designed to be merged FIRST so subsequent perf PRs see the new
scenarios as already-existing baselines in CodSpeed.

## CodSpeed scenarios (perf framework)

* **X7 — bytes round-trip RECV** (1k / 16k / 256k). Each call
  round-trips `ByteBuffer.array()` over the wire and routes through
  OUTPUT_CONVERTER[BYTES_TYPE] -> decode_bytearray. Without X7 the
  decode_bytearray path is a measurement blind spot (every prior
  macro returns int / list / void / callback). X7-16k joins the
  CodSpeed CI scenario list.

* **X8 — bytes SEND** (1k / 16k / 256k). Complements X7 on the
  encode_bytearray + Python -> Java direction via
  ByteArrayOutputStream.write(byte[], int, int). X8-16k joins
  the CodSpeed list.

Together X7-16k and X8-16k cover both halves of the byte-codec
bandwidth surface that's Nagle-sensitive on Linux.

## perf_coverage_test.py (run with `pytest -v -s`)

* **ColdStartLatencyTest (issue py4j#557)** — three measurements with
  progressively more of the cold path excluded so a regression can
  be localized:
  - `test_full_cold_start` — fresh `java` subprocess + listening-
    socket wait + py4j handshake + first call. Includes JVM class
    loading; issue py4j#557's reported window. Caps <30 s.
  - `test_py4j_handshake_under_500ms` — JVM is already listening;
    only `JavaGateway()` + first call is timed. Isolates py4j's
    own contribution (<500 ms).
  - `test_first_vs_warm_call_ratio` — first call vs warm median,
    catches regressions in protocol-cache fill.

* **StringArgLatencyCurveTest** — per-call latency at 16 / 256 /
  4K / 16K / 64K byte string args. Fills the gap between M2b
  (~5 B arg) and X7-class large payloads. Surfaces the Nagle
  knee around the BufferedWriter buffer boundary (~8 K) on Linux.

* **BandwidthSummaryTest** — MB/s at 1K / 16K / 256K for both
  directions (recv via ByteBuffer.array, send via BAOS.write).
  Reports the numbers; asserts >0.1 MB/s floor to catch
  protocol-layer regressions without flaking on hardware variance.

## Validation handle for the open PRs

* PR #5 (decode_bytearray) — `BandwidthSummaryTest` recv MB/s
  should jump substantially after the list-comprehension fix.
  X7-16k drops on the CodSpeed dashboard.
* PR #6 (Nagle) — `StringArgLatencyCurveTest` flattens across
  the 8 K boundary; X7-16k and X8-16k drop dramatically on
  Linux CodSpeed runners (issue py4j#516's 4.4s / 100 calls
  reproducer is exactly X7-16k's pattern).

## Local sample output (macOS loopback)

```
[full-cold-start] subprocess.Popen -> first call: 0.31 s
[py4j-handshake-only] gateway+first-call: 1.4 ms
[first-vs-warm] first: 196us  warm-median: 108.4us  ratio: 1.8x
[string-arg latency curve]
       16 B arg:    361.2 us/call
      256 B arg:    270.6 us/call
     4096 B arg:    339.6 us/call
    16384 B arg:    387.6 us/call
    65536 B arg:    462.9 us/call
[bandwidth summary]
       size    recv MB/s    send MB/s
      1024B          7.1          6.8
     16384B         60.3         79.3
    262144B         94.1        244.7
```

macOS loopback shows no Nagle penalty (Linux CodSpeed runners will);
send is ~2.5x faster than recv at 256K because recv pays the
base64-decode + list-comprehension cost in decode_bytearray that
PR #5 fixes.

Co-authored-by: Isaac
@Tagar Tagar changed the title perf: decode_bytearray (~7.5x on bytes recv path) + X7 macro baseline perf: drop byte-list intermediate in decode_bytearray (closes #570) May 21, 2026
@Tagar
Tagar force-pushed the perf-quick-wins branch from 6651d5e to b315c30 Compare May 21, 2026 14:23
Tagar added a commit that referenced this pull request May 21, 2026
…ge (#7)

This PR establishes a measurement baseline that the open optimization
PRs (decode_bytearray #5, Nagle #6) can be validated against, plus
adds regression-guard coverage for several blind spots.

Designed to be merged FIRST so subsequent perf PRs see the new
scenarios as already-existing baselines in CodSpeed.

## CodSpeed scenarios (perf framework)

* **X7 — bytes round-trip RECV** (1k / 16k / 256k). Each call
  round-trips `ByteBuffer.array()` over the wire and routes through
  OUTPUT_CONVERTER[BYTES_TYPE] -> decode_bytearray. Without X7 the
  decode_bytearray path is a measurement blind spot (every prior
  macro returns int / list / void / callback). X7-16k joins the
  CodSpeed CI scenario list.

* **X8 — bytes SEND** (1k / 16k / 256k). Complements X7 on the
  encode_bytearray + Python -> Java direction via
  ByteArrayOutputStream.write(byte[], int, int). X8-16k joins
  the CodSpeed list.

Together X7-16k and X8-16k cover both halves of the byte-codec
bandwidth surface that's Nagle-sensitive on Linux.

## perf_coverage_test.py (run with `pytest -v -s`)

* **ColdStartLatencyTest (issue py4j#557)** — three measurements with
  progressively more of the cold path excluded so a regression can
  be localized:
  - `test_full_cold_start` — fresh `java` subprocess + listening-
    socket wait + py4j handshake + first call. Includes JVM class
    loading; issue py4j#557's reported window. Caps <30 s.
  - `test_py4j_handshake_under_500ms` — JVM is already listening;
    only `JavaGateway()` + first call is timed. Isolates py4j's
    own contribution (<500 ms).
  - `test_first_vs_warm_call_ratio` — first call vs warm median,
    catches regressions in protocol-cache fill.

* **StringArgLatencyCurveTest** — per-call latency at 16 / 256 /
  4K / 16K / 64K byte string args. Fills the gap between M2b
  (~5 B arg) and X7-class large payloads. Surfaces the Nagle
  knee around the BufferedWriter buffer boundary (~8 K) on Linux.

* **BandwidthSummaryTest** — MB/s at 1K / 16K / 256K for both
  directions (recv via ByteBuffer.array, send via BAOS.write).
  Reports the numbers; asserts >0.1 MB/s floor to catch
  protocol-layer regressions without flaking on hardware variance.

## Validation handle for the open PRs

* PR #5 (decode_bytearray) — `BandwidthSummaryTest` recv MB/s
  should jump substantially after the list-comprehension fix.
  X7-16k drops on the CodSpeed dashboard.
* PR #6 (Nagle) — `StringArgLatencyCurveTest` flattens across
  the 8 K boundary; X7-16k and X8-16k drop dramatically on
  Linux CodSpeed runners (issue py4j#516's 4.4s / 100 calls
  reproducer is exactly X7-16k's pattern).

## Local sample output (macOS loopback)

```
[full-cold-start] subprocess.Popen -> first call: 0.31 s
[py4j-handshake-only] gateway+first-call: 1.4 ms
[first-vs-warm] first: 196us  warm-median: 108.4us  ratio: 1.8x
[string-arg latency curve]
       16 B arg:    361.2 us/call
      256 B arg:    270.6 us/call
     4096 B arg:    339.6 us/call
    16384 B arg:    387.6 us/call
    65536 B arg:    462.9 us/call
[bandwidth summary]
       size    recv MB/s    send MB/s
      1024B          7.1          6.8
     16384B         60.3         79.3
    262144B         94.1        244.7
```

macOS loopback shows no Nagle penalty (Linux CodSpeed runners will);
send is ~2.5x faster than recv at 256K because recv pays the
base64-decode + list-comprehension cost in decode_bytearray that
PR #5 fixes.

Co-authored-by: Isaac
@Tagar
Tagar force-pushed the perf-quick-wins branch from b315c30 to 10f05d3 Compare May 21, 2026 16:10
@Tagar Tagar changed the title perf: drop byte-list intermediate in decode_bytearray (closes #570) perf: decode_bytearray (closes #570) + hot-path smart_decode shortcuts (credit @markjm #575) May 21, 2026
Tagar added 2 commits May 21, 2026 11:02
…j#570)

Per @PaperTsar's analysis in issue py4j#570, the list comprehension
[b for b in standard_b64decode(...)] allocates one PyObject per byte
and then reconstructs bytes from that list — pure overhead now that
Python 2 is no longer a target. standard_b64decode returns bytes
directly; the bytes() wrapper preserves the contract.

~7.5x speedup on 256KB payloads (microbench); CodSpeed run in this PR
gives the authoritative scenario-level number.

Closes py4j#570.

Co-authored-by: Isaac
Builds on the decode_bytearray fix in the previous commit by adopting
the hot-path perf shortcuts from @markjm's PR py4j#575 — but
carefully, avoiding the bytes/str regression that caused testGatewayAuth
to fail on that PR (see review at py4j#575 (comment) 4484469752).

## What's adopted from py4j#575

* **socket recv decoded at the source** — replace
  ``smart_decode(self.stream.readline()[:-1])`` with
  ``self.stream.readline()[:-1].decode("utf-8")`` at every hot
  read site:
    * ``java_gateway.py``: ``GatewayConnection.send_command`` (every
      JavaGateway round-trip), ``do_client_auth`` (auth handshake),
      4 sites in the callback-server ``run`` / ``_call_proxy`` /
      ``_get_params`` paths, and ``OutputConsumer.run`` (stdout
      consumer thread).
    * ``clientserver.py``: 7 sites in ClientServer send/recv +
      proxy command + ``_get_params``.
  Each saves the smart_decode dispatch (one isinstance call per read);
  on the per-call path this compounds quickly.

* **encode_float**: drop ``smart_decode(repr(float))`` — ``str(float)``
  returns the same shortest-roundtrip repr on Python 3, no dispatch.

* **str() in cold sites**: replace ``smart_decode(addr/port/id)`` with
  ``str(...)`` for finalizer-key construction, logging of
  getsockname(), and python-proxy-id generation. These were never
  bytes inputs; smart_decode was always doing the str() fallback.

* **launch_gateway** auth-token: decode at the source
  (``proc.stdout.readline()[:-len(os.linesep)].decode("utf-8")``).
  Without this, the rest of the auth flow compares bytes to str
  and silently fails authentication.

## What's deliberately NOT adopted from py4j#575

* **smart_decode in ``escape_new_line`` is KEPT.** Dropping it
  (as py4j#575 proposed) makes ``bytes.replace("str", "str")`` raise
  TypeError. This was the testGatewayAuth regression I flagged on
  py4j#575. The replace chain is fast enough that the
  smart_decode dispatch isn't a hot-path concern, and the safety
  net is load-bearing for any caller that hands escape_new_line
  bytes (auth tokens, legacy callers). Docstring updated to make
  the rationale explicit so a future refactor doesn't accidentally
  drop it.

* **decode_bytearray return type stays ``bytes`` (not ``bytearray``).**
  py4j#575 changed it to ``bytearray``, but the original (and current)
  return contract was ``bytes`` (via the ``bytearray2 = bytes`` alias
  on Python 3). A return-type change risks breaking downstream
  consumers (e.g. PySpark's binary deserialization). The
  decode_bytearray fix from issue py4j#570 is already in place from the
  previous commit; this commit doesn't touch it.

## Tests added

* **EscapeNewLineBytesInputSafetyTest** (4 tests) — pin the
  bytes-input contract on escape_new_line. ASCII bytes / str /
  UTF-8 bytes / empty bytes all flow through cleanly. If a future
  refactor drops the smart_decode safety net, these tests fail
  immediately — catching the regression before the JVM-bound
  testGatewayAuth integration test would.

## Existing integration tests that validate this change

* ``GatewayLauncherTest.testGatewayAuth`` (java_gateway_test.py) —
  exercises ``launch_gateway(enable_auth=True)`` end-to-end. The
  bytes-stream auth-token read passes through the new
  ``.decode("utf-8")`` path.
* ``PythonEntryPointTest.test_python_entry_point_with_auth``
  (java_callback_test.py) — exercises the callback-with-auth path
  through all 4 modified read sites in the callback server.

Both pass on the full Python x Java x OS matrix.

Credit to @markjm for the perf shortcuts in py4j#575.

Co-authored-by: Isaac
@Tagar
Tagar force-pushed the perf-quick-wins branch from 469fe44 to e7b1485 Compare May 21, 2026 17:02
@Tagar
Tagar merged commit eaa7c9c into master May 21, 2026
58 checks passed
Tagar added a commit that referenced this pull request May 21, 2026
…03)"

This reverts commit 5026653 — removing FindBugs was overreach on
weak evidence:

* The 403 was on a SINGLE cell in PR #9's matrix (Python 3.9 /
  Java 8 / ubuntu-latest). Other cells in the SAME matrix run
  resolved `findbugs:3.0.+` successfully — proving the 403 is
  transient (likely Maven Central IP-throttling fresh runners),
  not a permanent policy change.
* Master CI on PRs #4 / #5 / #6 / #7 / #8 has been passing the
  same FindBugs resolution step reliably for months.
* Removing static analysis to "fix" a single flake degrades code
  quality on every future build.

The right defensive measures are already in this PR:
* shell-level retry around `./gradlew check && assemble`
* `shell: bash` for cross-platform consistency

If FindBugs ever does become permanently unavailable, that's the
moment to switch to SpotBugs — a real plugin migration, not a
panic delete.

Co-authored-by: Isaac
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Performance Issue in decode_bytearray

1 participant