Uploaded image for project: 'WildFly'
  1. WildFly
  2. WFLY-15343

More efficient/cleaner use of Maps

    XMLWordPrintable

Details

    • Enhancement
    • Resolution: Unresolved
    • Minor
    • None
    • None
    • None
    • None
    • Low
    • Regression

    Description

      There are multiple places within the repository that are not using Maps in the most efficient or clearest way. The issues seen are:

       

      (1) Using map.keySet() instead of map.entrySet()

      In many loops #keyset() is being used to iterate through a Map, but the key is then used with a #get() call to retrieve an element from the same Map. It is more efficient to use #entrySet(), which allows you to iterate over both key and value rather than needing to make a new retrieval call.

      For example, the following code in testsuite/shared/src/main/java/org/jboss/as/test/integration/common/jms/ActiveMQProviderJMSOperations.java:

      for (String key : params.keySet()) {
          model.get("params").add(key, params.get(key));
      }
      

      Could be replaced with:

      for (Map.Entry<String, String> entry : params.entrySet()) {
          model.get("params").add(entry.getKey(), entry.getValue());
      }
      

       

      (2) Using map.keySet().contains() instead of map.containsKey()

      Retrieving the keyset just to do a contains call is unnecessary, a method containsKey() exists which performs the same action. Very little performance difference for this one, just simpler code.

      For example, the following code in testsuite/integration/xts/src/test/java/org/jboss/as/test/xts/wsba/participantcompletion/service/BAParticipantCompletionSuperService.java:

      if (participantRegistry.keySet().contains(txid) &&
      

      Could be replaced with:

      if (participantRegistry.containsKey(txid) &&
      

       

      (3) Potentially inefficient use of map.keySet().removeAll(Collection c)

      When the collection to be removed from the keySet is smaller than the size of the keySet itself, this call will work as expected (iterating over each element in the collection, removing it from the keyset). However, if the collection to be removed is larger, then each element of the keyset is checked against the collection, and removed if it exists. This means an extra collection.contains() call is made for each element in the keyset. It is quicker in this second case to instead iterate over the collection and remove from the keyset one by one.

      In the only instance of this I noticed, the following code in clustering/server/src/main/java/org/wildfly/clustering/server/dispatcher/ChannelCommandDispatcherFactory.java:

      this.members.keySet().removeAll(leftMembers);
      

      Could be replaced with:

      leftMembers.forEach(this.members.keySet()::remove);
      

      Attachments

        Activity

          People

            mjusko@redhat.com Marek Jusko
            zodac01 Arouge Mehdi (Inactive)
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

              Created:
              Updated: