java - loop - Advertencia de FindBugs: uso ineficiente de keySet iterator en lugar de entrySet iterator

loop map

Question

Por favor, consulte el siguiente método:

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 da este mensaje waring:

" Uso ineficiente de keySet iterator en lugar de entrySet iterator Este método accede al valor de una entrada del Mapa, utilizando una clave que se recuperó de un iterador keySet. Es más eficiente usar un iterador en el inputSet del mapa, para evitar el Mapa .get (clave) de búsqueda ".

Answer 1

En conjunto de claves, debe obtener todas las claves y luego buscar todas las claves de la colección.

Además, el bucle sobre el inputSet es más rápido, porque no consulta el mapa dos veces para cada clave.

Si solo necesita claves o solo los valores de su Mapa, entonces use más bien KeySet () o valores ().

Answer 2

Está obteniendo el conjunto de claves en el mapa, y luego usa cada clave para obtener el valor fuera del mapa.

En su lugar, simplemente puede recorrer los Map.Entry clave / valor Map.Entry devuelven a través de entrySet() . De esa manera, evitará la búsqueda relativamente costosa de get() (tenga en cuenta el uso de la palabra relativamente aquí)

p.ej

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

Answer 3

Está recuperando todas las claves (accediendo a todo el mapa) y luego, para algunas claves, vuelve a acceder al mapa para obtener el valor.

Puede iterar sobre el mapa para obtener las entradas del mapa ( Map.Entry ) (parejas de claves y valores) y acceder al mapa solo una vez.

Map.entrySet() entrega un conjunto de Map.Entry s cada uno con la clave y el valor correspondiente.

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

Nota : dudo que esto sea una gran mejora ya que si usa entradas de mapas, creará una instancia de un objeto para cada entrada. No sé si esto es realmente más rápido que llamar a get() y recuperar la referencia necesaria directamente.

Answer 4

Esta es la sugerencia; No es realmente una respuesta para tu pregunta. Cuando estás trabajando con ConcurrentHashMap; A continuación se muestra el comportamiento del iterador mencionado en javadoc.

El iterador de la vista es un iterador "débilmente consistente" que nunca lanzará ConcurrentModificationException, y garantiza atravesar los elementos tal como existían en la construcción del iterador, y puede ( pero no está garantizado ) reflejar cualquier modificación posterior a la construcción.

Así que si usas el iterador EntrySet; esto puede contener par clave / valor obsoleto; así sería mejor; obtener la clave de keySet iterator (); y consultar con la recaudación de valor. Esto asegurará que esté recibiendo el cambio reciente de la colección.

Si está de acuerdo con el iterador de seguridad; entonces revisa este link ; se establece utilizando entrySet; Poco mejorando el rendimiento.

Answer 5

Si alguien todavía está interesado en una respuesta detallada y con respaldo numérico: sí, debe usar entrySet() frente a keySet() en caso de que esté iterando en todo el Mapa. Vea este Gist para los números detallados. Ejecuto un punto de referencia con JMH para las implementaciones predeterminadas de Map con Oracle JDK8.

El principal hallazgo es: siempre es un poco más lento para iterar sobre keySet y volver a consultar para cada clave. Tan pronto como tenga mapas más grandes, el multiplicador puede volverse bastante grande (por ejemplo, para un ConcurrentSkipListMap siempre es 5-10x; mientras que para HashMap s no es más grande que 2x para un millón de entradas).

Sin embargo, estos son todavía números muy pequeños. La forma más lenta de iterar más de 1 millón de entradas es con un ConcurrentSkipListMap.keySet() , que es de alrededor de 500-700 milisegundos; mientras que la iteración sobre IdentityHashMap.entrySet() es solo de 25 a 30 milisegundos con LinkedHashMap.entrySet() justo detrás con 40-50 milisegundos (no es sorprendente, ya que tiene una lista LinkedList en su interior que ayuda con la iteración). Como una visión general de la Gist enlazada arriba:

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

Así que la conclusión es: depende de su caso de uso. Si bien es definitivamente más rápido iterar sobre el entrySet() los números no son enormes, especialmente para mapas razonablemente pequeños. Sin embargo, si está iterando sobre un Mapa con 1 millón de entradas con bastante regularidad, mejor use la forma más rápida;)

Los números, por supuesto, son solo para comparar unos con otros, no absolutos.