Skip to content

[SPARK-59617][SQL] Reuse a per-evaluator output buffer in JsonExpressionEvalUtils (json_tuple, JSON_TABLE) - #58889

Open
david-mollitor-db wants to merge 1 commit into
apache:masterfrom
david-mollitor-db:json-eval-reuse-outputbuffer
Open

david-mollitor-db wants to merge 1 commit into
apache:masterfrom
david-mollitor-db:json-eval-reuse-outputbuffer

Conversation

@david-mollitor-db

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

JsonExpressionEvalUtils has several JSON-extraction evaluators. Two of them --
GetJsonObjectEvaluator and MultiGetJsonObjectEvaluator -- already reuse a single
@transient private lazy val outputBuffer (a ByteArrayOutputStream) across rows, calling
reset() before each use. The other two allocate a fresh ByteArrayOutputStream on every
field/element:

  • JsonTupleEvaluator.parseRow -- a new buffer per extracted field (unbounded per row for a wide
    json_tuple(...)).
  • JsonTableEvaluator.serializeCurrentValue -- a new buffer per call; the hot caller expands a
    JSON array one element at a time.

This extends the existing outputBuffer reuse pattern to those two evaluators: each gets an
instance outputBuffer field and calls reset() before writing to / reading from it.

Why are the changes needed?

A no-arg ByteArrayOutputStream starts at the JDK default of 32 bytes and grows by doubling;
allocating (and then GC-ing) a fresh one per field/element is steady allocation churn on the
json_tuple / JSON_TABLE eval path. Reusing one buffer per evaluator -- already the pattern for
the two get-json-object evaluators in the same file -- removes it.

Does this PR introduce any user-facing change?

No. Each value's bytes are fully materialized into a UTF8String
(UTF8String.fromBytes(outputBuffer.toByteArray)) before the buffer is reused, so the output is
byte-for-byte identical. The evaluators are per-expression instances evaluated one row at a time
(GenerateExec drains each row's iterator sequentially), so the reused buffer is never shared
concurrently or used re-entrantly.

How was this patch tested?

Existing JsonExpressionsSuite (json_tuple and the get-json-object evaluators) and
JsonTableSuite (JSON_TABLE row expansion, including multi-element arrays -- the reused-buffer hot
path) pass. This is a behavior-preserving change, so no new tests were added.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Isaac

This pull request and its description were written by Isaac.

…ionEvalUtils (json_tuple, JSON_TABLE)

`JsonExpressionEvalUtils` has several JSON-extraction evaluators. `GetJsonObjectEvaluator` and
`MultiGetJsonObjectEvaluator` already reuse a single `@transient private lazy val outputBuffer`
(a `ByteArrayOutputStream`), calling `reset()` before each use. The other two allocated a fresh
`ByteArrayOutputStream` on every field/element:
- `JsonTupleEvaluator.parseRow` -- one per extracted field (unbounded per row for a wide
  `json_tuple(...)`).
- `JsonTableEvaluator.serializeCurrentValue` -- one per call; the hot caller expands a JSON array
  one element at a time.

Extend the existing `outputBuffer` pattern to those two: each gets a reused instance field and
`reset()`s it before each use. A no-arg `ByteArrayOutputStream` starts at 32 bytes and grows by
doubling, so this removes a fresh allocation (and its GC) per field/element on the json_tuple /
JSON_TABLE eval path.

Behavior-preserving: each value's bytes are materialized into a `UTF8String` before the buffer is
reused, so output is byte-for-byte identical; the evaluators are per-expression instances evaluated
one row at a time, so the reused buffer is never shared concurrently or used re-entrantly.

Verified with `JsonExpressionsSuite` and `JsonTableSuite`.

Co-authored-by: Isaac <no-reply@databricks.com>
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