khaledh opened a new issue, #40188: URL: https://github.com/apache/beam/issues/40188
### What would you like to happen? `TableRowToStorageApiProto.messageFromMap` resolves every named `TableRow` field, at every nesting level, with `Descriptor.findFieldByName`. In protobuf-java that call concatenates the message's full name with the field name and looks the result up in the file's symbol table. For large, deeply nested rows this repeats a string allocation and a long-string hash for every field of every record. On a production Dataflow pipeline writing rows with 1,059 schema fields and nesting depth 10 through `BigQueryIO.write()` with `STORAGE_WRITE_API`, a JFR profile of the conversion step attributed about 36% of its allocation to `findFieldByName` inside the recursive `messageFromMap`. ### Proposal Record each field's ordinal position while building the immutable `SchemaInformation` tree. During conversion, call `descriptor.getFields()` once per message and use the ordinal as a lookup hint: if the descriptor field at that position has the expected name, use it; otherwise fall back to the existing `findFieldByName`. Descriptors produced by `getDescriptorFromTableSchema` preserve schema order, so the hint hits in the normal case, and reordered or otherwise compatible descriptors keep working through the fallback. This adds no cache, no shared mutable state, no configuration, and no public signature change. Required-field validation and all diagnostics are unchanged. ### Measurements JMH, synthetic schema with 10 nested levels, 16 repeated rows per level, 10 primitive fields per message (JDK 17): | Version | Average time | |---|---:| | master | 1.575 ± 0.063 ms/op | | with ordinal hint | 1.084 ± 0.007 ms/op | That is a 31.2% reduction in conversion time. A GC-profiler run measured allocation of 3,971,036 → 3,476,761 B/op (−12.4%). On the production-shaped workload above, a paired Dataflow comparison at equal, fixed capacity (35 × n2-standard-4, Streaming Engine) measured 15.7% lower CPU-seconds per million records with this change alone, with byte-identical output. ### Related The same profile showed a second, independent cost in the same conversion: `DynamicMessage.Builder.build()` runs a full `isInitialized()` walk on every message even when the caller passed `allowMissingRequiredFields=true` and the descriptor has no required fields, so the walk cannot fail. That will be filed and addressed in a separate issue and PR. On the production workload above, the two changes together reduced CPU per record by 33.6% and increased drain throughput by 46% at fixed capacity. A PR implementing this issue is ready. A JMH benchmark module for `TableRowToStorageApiProto` will follow as a third, separate PR. ### Issue Priority Priority: 2 (default / most feature requests should be filed as P2) ### Issue Components - [ ] Component: Python SDK - [x] Component: Java SDK - [ ] Component: Go SDK - [ ] Component: Typescript SDK - [x] Component: IO connector - [ ] Component: Beam YAML - [ ] Component: Beam examples - [ ] Component: Beam playground - [ ] Component: Beam katas - [ ] Component: Website - [ ] Component: Infrastructure - [ ] Component: Spark Runner - [ ] Component: Flink Runner - [ ] Component: Prism Runner - [ ] Component: Twister2 Runner - [ ] Component: Hazelcast Jet Runner - [ ] Component: Google Cloud Dataflow Runner -- 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]
