[GitHub] [flink] twalthr commented on pull request #14420: [FLINK-19981][core][table] Add name-based field mode for Row

[GitBox <gi...@apache.org>]

twalthr commented on pull request #14420:
URL: https://github.com/apache/flink/pull/14420#issuecomment-753971961


   I added a commit for with benchmarks in this PR. I will remove it again during merging. As expected, the name-based rows perform 35 % worse then position-based rows. But I think for the sake of code readability and not having to deal with indices, this is acceptable. Users can always fallback to position-based fields. I also added a comment to the JavaDoc about this.
   
   Here are the benchmark results:
   
   ```
   RowBenchmark.testBefore                   thrpt   45  2.559 ± 0.020  ops/s        // before this PR
   RowBenchmark.testNamed                    thrpt   45  1.632 ± 0.014  ops/s
   RowBenchmark.testPositioned               thrpt   45  2.653 ± 0.014  ops/s
   RowBenchmark.testNamedNotCopy             thrpt   45  2.886 ± 0.042  ops/s
   RowBenchmark.testPositionedNotCopy        thrpt   45  4.441 ± 0.013  ops/s
   RowBenchmark.testPojo                     thrpt   45  2.904 ± 0.027  ops/s
   RowBenchmark.testPositionedSerialization  thrpt   45  4.591 ± 0.030  ops/s
   RowBenchmark.testNamedSerialization       thrpt   45  3.396 ± 0.020  ops/s
   ```
   
   Interestingly, the new refactoring of the serializer seems to slightly improve the position-based rows as well. This might be due to a modified `RowSerializer.deserialize`.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org