bvolpato commented on PR #39619:
URL: https://github.com/apache/beam/pull/39619#issuecomment-6070207701
Yes, I think there is. The class generated today specializes very little: it
holds the schema and `hasNullableFields` as constants, and `encode` and
`decode` forward to the same shared loops over `Coder[]` that this PR keeps.
Every schema goes through the same `coders[i].encode(...)` call sites.
A generator that really specializes could emit one call per field, with the
component coder as a constant of its exact type and the row index hard-coded.
It could also resolve the static-encoding option at generation time (today that
is a map lookup per row).
To size the opportunity, I wrote by hand what such a generator could emit
for the two-field benchmark schemas and compared it with a copy of the loops.
This code is not in the PR.
| Operation | Nested | Static | Run 1 | Run 2 |
| --- | --- | --- | ---: | ---: |
| decode | false | false | -0.3% | -3.4% |
| decode | false | true | -18.4% * | -13.4% * |
| decode | true | false | -6.2% | -4.6% |
| decode | true | true | -22.5% * | -35.1% * |
| encode | false | false | -3.0% | -4.4% |
| encode | false | true | -13.1% * | -10.9% * |
| encode | true | false | -14.1% | -13.9% * |
| encode | true | true | -43.5% * | -31.4% * |
Change in ns/op of the straight-line code against the loop. Same JMH
protocol as the PR description. `*` marks non-overlapping 99.9% intervals.
Please read this as directional only. Other builds were running on the
machine during both runs, the schemas have two fields and no nullable fields,
and this is hand-written Java, not generated bytecode. The pattern does repeat
across both runs: no clear gain for flat rows with dynamic encoding, about 5%
to 14% for nested rows with dynamic encoding, and about 11% to 44% with static
encoding.
I also tried the cheap part without codegen: the existing loop, with the
static-encoding option resolved once at construction. That gave 8% to 9% for
static encoding on the encode side in one run and nothing clear elsewhere. So
most of the difference seems to come from the per-field call sites. I did not
profile to confirm that.
Limits I would expect:
- Wide schemas. By default HotSpot does not JIT-compile a method above 8,000
bytes of bytecode (`DontCompileHugeMethods`), and a method cannot exceed 64 KB.
By my estimate straight-line code needs more than 20 bytes per field, so a
1,024-field schema must be split into chunks. I have not measured that case.
- Generation cost. More bytecode per schema makes coder creation slower than
it is today, which is the opposite direction of this PR.
- Semantics. A baked-in static-encoding flag assumes that the row has the
same schema options as the coder. Today `encode` reads the option from the
schema of the row.
So the two directions conflict. This PR drops the generated class to make
creation cheaper, and specialized codegen would spend more at creation to make
steady state faster. It would be a separate change, built on the existing
generator and not on this PR.
<details>
<summary>Straight-line coder used for the flat schema (BYTE field0, BYTES
field1)</summary>
```java
static final class FlatStraightCoder extends CustomCoder<Row> {
private static final ByteCoder FIELD0_CODER = ByteCoder.of();
private static final ByteArrayCoder FIELD1_CODER = ByteArrayCoder.of();
private final Schema schema;
private final boolean staticEncoding;
FlatStraightCoder(Schema schema) {
this.schema = schema;
this.staticEncoding =
schema.getOptions().getValueOrDefault("beam:option:row:static_encoding", false);
}
@Override
public void encode(Row value, OutputStream outputStream) throws
IOException {
checkState(value.getFieldCount() == 2);
if (!staticEncoding) {
VAR_INT_CODER.encode(2, outputStream);
NULL_LIST_CODER.encode(EMPTY_BIT_SET, outputStream);
}
Byte field0 = value.getValue(0);
if (field0 != null) {
FIELD0_CODER.encode(field0, outputStream);
}
byte[] field1 = value.getValue(1);
if (field1 != null) {
FIELD1_CODER.encode(field1, outputStream);
}
}
@Override
public Row decode(InputStream inputStream) throws IOException {
Object[] fieldValues = new Object[2];
if (staticEncoding) {
fieldValues[0] = FIELD0_CODER.decode(inputStream);
fieldValues[1] = FIELD1_CODER.decode(inputStream);
} else {
// The field count and the null list still allow compatible schema
changes.
int fieldCount = VAR_INT_CODER.decode(inputStream);
BitSet nullFields = NULL_LIST_CODER.decode(inputStream);
if (fieldCount > 0 && !nullFields.get(0)) {
fieldValues[0] = FIELD0_CODER.decode(inputStream);
}
if (fieldCount > 1 && !nullFields.get(1)) {
fieldValues[1] = FIELD1_CODER.decode(inputStream);
}
}
return Row.withSchema(schema).attachValues(fieldValues);
}
}
```
The nested variant has the same shape, with two `FlatStraightCoder` fields
in place of the byte coders. The benchmark setup checks that each variant
writes the same bytes as `RowCoder.of(schema)`.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
