Uploaded image for project: 'OpenAM'
  1. OpenAM
  2. OPENAM-13011

SessionMonitor should avoid use of ConcurrentHashMap.computeIfAbsent

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Won't Fix
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: monitoring, session
    • Labels:
    • Target Version/s:
    • Needs backport:
      No
    • Needs QA verification:
      No
    • Functional tests:
      No
    • Are the reproduction steps defined?:
      No (add reasons in the comment)

      Description

      Bug description

      While https://bugs.openjdk.java.net/browse/JDK-8161372 still affects JDK 8, we should avoid use of ConcurrentHashMap.computeIfAbsent.

      com.iplanet.dpro.session.service.SessionMonitor methods which are called very often use computeIfAbsent. These methods should be updated to avoid use of that method.

      How to reproduce the issue

      All session operations pass through methods of com.iplanet.dpro.session.service.SessionMonitor which call ConcurrentHashMap.computeIfAbsent. Avoiding use of computeIfAbsent may improve AM performance across all performance tests.

      Expected behaviour
      Session monitoring has little impact on performance.
      
      Current behaviour
      It's possible that session monitoring's impact on performance is greater than desired.
      

      Work around

      Update SessionMonitor to avoid using computeIfAbsent. For example, we could instead call ConcurrentHashMap.get and, only if nothing is returned, obtain a lock from org.forgerock.openam.shared.concurrency.LockFactory while lazily creating the desired metric object and putting it into the map.

      e.g.

      com.iplanet.dpro.session.service.SessionMonitor.java
          private DistributionSummary getOrCreateSummary(String metricSessionType, String operation) {
              String key = metricSessionType + ":" + operation;
              DistributionSummary summary = summaries.get(key);
              if (summary == null) {
                  Lock lock = lockFactory.acquireLock(key);
                  lock.lock();
                  try {
                      summary = summaries.get(key);
                      if (summary == null) {
                          summary = createSummary(metricSessionType, operation);
                          summaries.put(key, summary);
                      }
                  } finally {
                      lock.unlock();
                  }
              }
              return summary;
          }
      

        Attachments

          Activity

            People

            • Assignee:
              tom.elliott Tom Elliott [X] (Inactive)
              Reporter:
              craig.mcdonnell Craig McDonnell
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: