kinolaev commented on issue #11648: URL: https://github.com/apache/iceberg/issues/11648#issuecomment-4826628564
I believe I've found the root cause of the issue - hash code computation for each row of a position delete file. When the cache is disabled, `toPositionIndex` pushes the file_path predicate down to a delete file reader, which is the most efficient way to read the relevant rows. When the cache is enabled, the delete file is read fully, and for each row `toPositionIndexes` method calls `CharSequenceMap.computeIfAbsent` that requires a hash computation for the provided `file_path`. As the flame chart shows `computeIfAbsent` method takes at least half of the samples. The Iceberg spec states that "The rows in the delete file must be sorted by file_path then pos to optimize filtering rows while scanning". That means that we only need to lookup an index when `file_path` is different from the previous row. As far as I know `org.apache.commons.lang3.Strings.CS.equals()` should be cheaper than `org.apache.iceberg.util.CharSequenceWrapper.hashCode()`, so reusing index when `file_path` is the same between contiguous rows should improve performance of `toPositionIndexes` method. I've added the change to my PR (https://github.com/apache/iceberg/pull/15714, commit fa492a6ead4eeb06d0275511b32822c6cb8749f5). @davseitsev , could you please test if it works for you? @nastra , could you please reopen the issue? -- 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]
