SEZ9 commented on PR #12578: URL: https://github.com/apache/seatunnel/pull/12578#issuecomment-5976110144
Thanks @hutiefang76 for the detailed confirmation against ec14f3a. Your summary of the three items lines up with what I asked for: 1. **setField hot path (F1):** array store, one read in `if (size != 0)`, and a conditional `size = 0` write, so a fresh row pays no cache write. 2. **Contract placement / Javadoc (F2):** the paragraph moved into the shared `Row Data Contract` section of the EN/ZH architecture guides, with the same contract in the public setter / `getFields` / size-overload Javadoc. 3. **Test coverage (F3/F5):** `testDirectFieldMutationDoesNotInvalidateCachedBytesSize` and `testNestedFieldMutationDoesNotInvalidateParentCachedBytesSize` covering the documented non-invalidation of `getFields()` / nested-row mutation and that both overloads report the same cached value. I'll verify each of these against the diff at ec14f3a before closing them out. One remaining ask on F4: I don't see the cross-overload sequence spelled out — typed `getBytesSize(rowType)` pre-warm, then `setField`, then the untyped `getBytesSize()` on the *same* row returning the recomputed value. If one of the 13 tests already does this, just point me to its name; otherwise a short assertion in one of the existing tests is enough. Also appreciate not claiming a performance win given the overlapping JMH intervals — keeping the PR description to the correctness fix is the right call. <!-- streview-comment:1507 --> -- 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]
