Uploaded image for project: 'Magnolia'
  1. Magnolia
  2. MAGNOLIA-7994

Remove synchronisation block from ModuleInstanceProvider API

XMLWordPrintable

    • Icon: Improvement Improvement
    • Resolution: Done
    • Icon: Neutral Neutral
    • 6.2.14
    • None
    • None
    • Yes
    • Yes

       Problem

      public class ModuleInstanceProvider<T> implements Provider<T> {
      
      public synchronized T get() {
          T moduleInstance = (T) moduleRegistry.getModuleInstance(moduleName);
          if (moduleInstance == null) {
              throw new ProvisionException("..");
          }
          return moduleInstance;
      }
      

      code- the API unblocked

      • module registry to be changed to use `ConcurrentHashMap`

      Dev notes

      See the attached branch with the draft solution.
      Caveat: we used to use linked hash maps there, so with the above change we lose the order, so the more accurate fix is needed!

      Update

      Since apparently there is no HashMap implementation which is at the same time thread-safe and able to keep the insertion order, I tried to wrap LinkedHashMap in Collections.synchronizedMap but running again load tests did not show any improvement: the bottleneck now being shifted to Collections$SynchronizedMap.get. This is because synchronizedMap apparently synchronises reads too.

      Implemented solution

      Keep using LinkedHashSet and let ReentrantReadWriteLock handle synchronisation: this allows concurrent reads unless one writing thread is holding the lock.

      Ran a stress test against latest dx-core-demo 6.2.14-SNAPSHOT (as configured out of the box) and profiled Tomcat instance with JProfiler.
      The bottlenecks at ModuleInstanceProvider and ModuleRegistryImpl are gone and the throughput as measured by Requests/sec. ratio seems to have improved compared to before the changes.

      wrk -c200 -t200 -d600s --timeout 20s --latency --header "User-Agent: wrk" http://localhost:8080/magnoliaPublic/travel.htm
      
      Running 10m test @ http://localhost:8080/magnoliaPublic/travel.htm
        200 threads and 200 connections
       ...
        1015507 requests in 10.00m, 24.59GB read
        Socket errors: connect 0, read 0, write 0, timeout 113
      Requests/sec:   1692.15
      Transfer/sec:     41.95MB
      

       

        Acceptance criteria

              fgrilli Federico Grilli
              apchelintcev Aleksandr Pchelintcev
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Created:
                Updated:
                Resolved:

                  Task DoD