[ https://issues.apache.org/jira/browse/CASSANDRA-15394?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16963859#comment-16963859 ]
Benedict Elliott Smith edited comment on CASSANDRA-15394 at 10/31/19 12:38 PM: ------------------------------------------------------------------------------- LGTM. _Very optional suggestion_: If we're doing an optimisation pass, it might be worth extracting the call to {{.size()}}? Since, at least in cases where the concrete type of the {{List}} is opaque to the JVM, it will be unable to understand that it is constant. This includes at least {{EncodingStats.merge}} and {{MergeIterator.close}}, as these are megamorphic, given the wide variety of {{List}} type we use. e.g., {{for (int i=0,mi=list.size(); i<mi; i++)}} It would be nice to start a pattern for this in the codebase, so that it becomes a norm, at least until we clean up our megamorphic list usages, and this is as good a place as any given performance is the sole purpose. It also gives us a chance to discuss what a good standard variable name for this is. As I say, entirely optional, but also a pretty low cost kind of hygiene to introduce. was (Author: benedict): LGTM. _Very optional suggestion_: If we're doing an optimisation pass, it might be worth extracting the call to {{.size()}}? Since, at least in cases where the concrete type of the {{List}} is opaque to the JVM, it will be unable to understand that it is constant. This includes at least {{EncodingStats.merge}} and {{MergeIterator.close}}, as these are megamorphic, given the wide variety of {{List}} type we use. e.g., \{{ for (int i=0,mi=list.size(); i<mi; i++) }} It would be nice to start a pattern for this in the codebase, so that it becomes a norm, at least until we clean up our megamorphic list usages, and this is as good a place as any given performance is the sole purpose. It also gives us a chance to discuss what a good standard variable name for this is. As I say, entirely optional, but also a pretty low cost kind of hygiene to introduce. > Remove list iterators > --------------------- > > Key: CASSANDRA-15394 > URL: https://issues.apache.org/jira/browse/CASSANDRA-15394 > Project: Cassandra > Issue Type: Sub-task > Components: Local/Compaction > Reporter: Blake Eggleston > Assignee: Blake Eggleston > Priority: Normal > Fix For: 4.0 > > > We allocate list iterators in several places in hot paths. This converts them > to get by index. This provides a ~4% improvement in relvant workloads. -- This message was sent by Atlassian Jira (v8.3.4#803005) --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org