FindBugs 警告：使用 keySet 迭代器而不是 entrySet 迭代器效率低下答案

【问题标题】：FindBugs warning: Inefficient use of keySet iterator instead of entrySet iteratorFindBugs 警告：使用 keySet 迭代器而不是 entrySet 迭代器效率低下
【发布时间】：2012-09-20 07:03:49
【问题描述】：

请参考以下方法：

public Set<LIMSGridCell> getCellsInColumn(String columnIndex){
    Map<String,LIMSGridCell> cellsMap = getCellsMap();
    Set<LIMSGridCell> cells = new HashSet<LIMSGridCell>();
    Set<String> keySet = cellsMap.keySet();
    for(String key: keySet){
      if(key.startsWith(columnIndex)){
        cells.add(cellsMap.get(key));
      }
    }
    return cells;
  }

FindBugs 发出警告信息：

"低效使用keySet迭代器代替entrySet迭代器 这个方法使用一个键来访问一个 Map 条目的值，该键是从 keySet 迭代器中检索。使用一个更有效地图 entrySet 上的迭代器，避免 Map.get(key) 查找。”

【问题讨论】：

如果Map 是一个哈希映射，它是否更有效是有争议的，因为查找是O(1)，，否则它必须是一个@ 987654323@ 查找是 (O log N)。 这几乎不会有太大的不同。纯属吹毛求疵。

标签： java performance findbugs

【解决方案1】：

如果有人仍然对详细和数字支持的答案感兴趣：是的，您应该使用 entrySet() 与 keySet() 以防遍历整个地图。有关详细数字，请参阅this Gist。我使用 JMH 为使用 Oracle JDK8 的 Map 的默认实现运行了一个基准测试。

主要发现是：遍历keySet 并重新查询每个键总是有点慢。一旦你有更大的地图，乘数就会变得相当大（例如，对于ConcurrentSkipListMap，它始终是 5-10 倍；而对于 HashMaps，它不大于 2 倍，最多可达一百万个条目）。

但是，这些仍然是非常小的数字。迭代超过 100 万个条目的最慢方法是使用 ConcurrentSkipListMap.keySet()，大约为 500-700 毫秒；而对IdentityHashMap.entrySet() 的迭代只需 25-30 毫秒，LinkedHashMap.entrySet() 仅落后 40-50 毫秒（不足为奇，因为它内部有一个 LinkedList，这有助于迭代）。作为上述链接要点的概述：

Map type              | Access Type | Δ for 1M entries
----------------------+-------------+-----------------
HashMap               | .entrySet() |     69-72  ms
HashMap               |   .keySet() |     86-94  ms
ConcurrentHashMap     | .entrySet() |     72-76  ms
ConcurrentHashMap     |   .keySet() |     87-95  ms
TreeMap               | .entrySet() |    101-105 ms
TreeMap               |   .keySet() |    257-279 ms
LinkedHashMap         | .entrySet() |     37-49  ms
LinkedHashMap         |   .keySet() |     89-120 ms
ConcurrentSkipListMap | .entrySet() |     94-108 ms
ConcurrentSkipListMap |   .keySet() |    494-696 ms
IdentityHashMap       | .entrySet() |     26-29  ms
IdentityHashMap       |   .keySet() |     69-77  ms

所以底线是：这取决于您的用例。虽然迭代entrySet() 绝对更快，但数字并不大，尤其是对于相当小的地图。但是，如果您经常迭代包含 100 万个条目的 Map，最好使用更快的方法；)

数字当然只是相互比较，不是绝对的。

【讨论】：

【解决方案2】：

在keyset中，你需要获取所有的key，然后搜索集合中的每一个key。

此外，遍历 entrySet 更快，因为您不会为每个键查询两次映射。

如果您只需要 Map 的键或值，请使用 keySet() 或 values()。

【讨论】：

【解决方案3】：

您正在检索所有键（访问整个映射），然后对于某些键，您再次访问映射以获取值。

您可以遍历地图以获取地图条目 (Map.Entry)（一对键和值）并仅访问地图一次。

Map.entrySet() 传递一组Map.Entrys，每个都带有键和对应的值。

for ( Map.Entry< String, LIMSGridCell > entry : cellsMap.entrySet() ) {
    if ( entry.getKey().startsWith( columnIndex ) ) {
        cells.add( entry.getValue() );
    }
}

注意：我怀疑这将是一个很大的改进，因为如果您使用映射条目，您将为每个条目实例化一个对象。我不知道这是否真的比调用get() 并直接检索所需的引用更快。

【讨论】：

但在 hashMap O(1) 上不是 get() 吗？
@Geek：是的。请参阅我添加的注释。我怀疑 FindBugs 的建议是否真的有意义。实例化和 get() 都是 O(1)
地图可以存储条目（例如 Sun 的 HashMap 实现），因此不需要实例化。并且 get() 可能大于 O(1)，例如具有错误哈希函数的 TreeMap 或 HashMap。但你是对的，在大多数情况下它不会产生明显的影响。
@Matteo 你能看看我的回答吗？如果有任何 cmets，请告诉我。
“如果您使用映射条目，您将为每个条目实例化一个对象”——当然不是。大多数地图实现已经是条目的地图。最值得注意的是，当迭代 HashMap 时，条目实例与内部存储的条目对象相同。所以在Entry 上调用getValue（同样是setValue）是直接访问值，而在地图上调用get 意味着在键上调用hashCode，计算数组索引，然后调用equals至少在键上一次，以访问您在使用 entrySet() 时已经拥有的同一个条目对象。

【解决方案4】：

这是建议；不是你的问题的真正答案。当您使用 ConcurrentHashMap 时；下面是javadoc中提到的迭代器行为

视图的迭代器是一个“弱一致”的迭代器，它永远不会抛出 ConcurrentModificationException，并保证遍历构造迭代器时存在的元素，并且可以（但不保证）反映之后的任何修改建设。

所以如果你使用 EntrySet 迭代器；这可能包含陈旧的键/值对；所以会更好；从 keySet iterator() 获取密钥；并检查收藏价值。这将确保您从集合中获得最近的更改。

如果你对故障安全迭代器没问题；然后检查这个link；它使用 entrySet 声明；性能提升不大。

【讨论】：

【解决方案5】：

您正在获取映射中的一组键，然后使用每个键从映射中获取值。

相反，您可以简单地遍历通过 entrySet() 返回给您的 Map.Entry 键/值对。这样你就可以避免相对昂贵的get() 查找（注意这里使用了相对这个词）

例如

for (Map.Entry<String,LIMSGridCell> e : map.entrySet()) {
   // do something with...
   e.getKey();
   e.getValue();
}

【讨论】：

在这种情况下，地图实现是 HashMap 。 HashMap O(1) 不是 get() 吗？
@Geek：是的，但是使用 entrySet() 可以完全删除对get()的调用
O(1) 没有指定需要多长时间，只是它是恒定的
但是他没有通过 get() 访问 each 值。仅使用其键与条件匹配的那些。我认为选择哪种方式没有一般规则。这取决于匹配条件的键的比例。显然，FindBugs 无法检查。