aiborodin commented on code in PR #13340:
URL: https://github.com/apache/iceberg/pull/13340#discussion_r2165747315
##########
flink/v2.0/flink/src/main/java/org/apache/iceberg/flink/sink/dynamic/TableMetadataCache.java:
##########
@@ -220,37 +238,59 @@ SchemaInfo getSchemaInfo() {
*/
static class SchemaInfo {
private final Map<Integer, Schema> schemas;
- private final Map<Schema, Tuple2<Schema, CompareSchemasVisitor.Result>>
lastResults;
+ private final Cache<Schema, SchemaCompareInfo> lastResults;
Review Comment:
I reverted the cache to use a `LinkedHashMap`. I also added [this
benchmark](https://github.com/aiborodin/iceberg/blob/optimise-row-data-conversion/flink/v2.0/flink/src/jmh/java/org/apache/iceberg/flink/sink/dynamic/CacheBenchmark.java#L44)
to compare it with the caffeine implementation. Here, `LRUCache` is the
`LinkedHashMap` based cache.
As you suggested @pvary, the `LinkedHashMap` cache is almost twice as fast:
```
Benchmark Mode Cnt Score Error
Units
CacheBenchmark.testCaffeineCacheMaxSize_Get thrpt 5 929.031 ± 84.510
ops/s
CacheBenchmark.testCaffeineCacheMaxSize_Put thrpt 5 548.677 ± 12.191
ops/s
CacheBenchmark.testLRUCache_Get thrpt 5 1657.313 ± 71.981
ops/s
CacheBenchmark.testLRUCache_Put thrpt 5 1206.151 ± 112.609
ops/s
```
We should probably replace the caffeine cache
[here](https://github.com/apache/iceberg/blob/main/flink/v2.0/flink/src/main/java/org/apache/iceberg/flink/sink/dynamic/TableMetadataCache.java#L55)
as well.
Thank you for your review and valuable feedback! I hope we can get this in
soon.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
