8 Replies Latest reply on Jun 8, 2005 11:18 AM by belaban

    LockInterceptor.transactions - why a synch List?

      Hi;

      As part of my performance analysis work I'm doing on my app that is using JBossCache, I've noticed some concurrency problems that appear to be related to the LockInterceptor.transactions attribute.

      While I haven't run tests on this to confirm, my analysis of LockInterceptor leads me to believe that a synchronized ArrayList may not be the best choice based on how it's used.

      First off, I think that a Set would suit the access pattern better than a list. As it stands, calls to transactions.contains() in invoke() will require a linear scan, similarly for transactions.remove() in afterCompletion().

      Secondly, I would suspect that some sort of reader/writer lock would be preferable to the single level of locking offered by Collections.synchronized.

      Although it doesn't seem that one would ever expect the number of entries in LockInterceptor.transactions to get very large, my tests on high-end (32 CPU) machines is indicating there is noticeable thread contention due to the time a thread has to spend doing linear scans of the list while holding an exclusive lock.

      Any thoughts you might have on this would be appreciated.

      Thanks,

      Brian.

        • 1. Re: LockInterceptor.transactions - why a synch List?
          belaban

          Okay. While I don't share your concern about the exclusive lock, I kind of agree with the fact the contains() and remove() have a cost linear to the number of transactions.
          Actually, I don't think this is a big problem, b/c you usually don't have thousands of transactions present at any given time.

          Anyway, I change the ArrayList to a HashSet() where lookup is constant. I'm running the testsuite right now, and if it passes I will check in my changes. Note that this affects other classes as well: ReplicationInterceptor, CacheStoreInterceptor and LockInterceptor.

          • 2. Re: LockInterceptor.transactions - why a synch List?

            Changing this to a HashSet should solve my concerns about lock exclusivity as well since the more efficient algorithm means that even if the list grows large the time threads are competing for that lock will remain small.

            In terms of how big of a problem this is, as I said in my post, I haven't fully quantified it, but it is showing up on our app monitoring as a point of contention for threads. You'd be surprised how quickly small things like this cause threads to pile up.

            Thanks for the quick response in making these changes, and for incorporating my suggestions for my earlier 2 posts. I'll be trying these out later today and will let you know if I see any problems.

            Thanks and regards,

            Brian.

            • 3. Re: LockInterceptor.transactions - why a synch List?
              belaban

              Let me know what else you discover. We have performance tests on the roadmap, but haven't gotten to it yet.
              I'm perf testing JGroups first, then JBossCache.

              • 4. Re: LockInterceptor.transactions - why a synch List?

                Will do, we're hammering JBossCache pretty hard right now. Doing large scale single VM testing on a 32-way Unix system. Following that we'll be trying a 4 way cluster.

                On other note, I tried to pick up the changes you made to CVS and I don't see them. In fact, even if I look at http://cvs.sourceforge.net/viewcvs.py/jboss/JBossCache/src/org/jboss/cache/ it doesn't show your changes. Am I missing something?

                • 5. Re: LockInterceptor.transactions - why a synch List?

                  Please disregard my last post re CVS, I found the new jforge stuff and am now synched up with the changes you checked in.

                  Brian.

                  • 6. Re: LockInterceptor.transactions - why a synch List?
                    belaban

                    Note that when you use replication, you want to use fc-fast.xml (from the JGroups distro, but I can also send it to you), rather than the default properties. The default properties tolerate bursty traffic well, but they are not suited for high sustained throughput.

                    • 7. Re: LockInterceptor.transactions - why a synch List?

                      OK, thanks. I'll try to find fc-fast.xml myself. As a small suggestion it might make sense for you to check-in a JBossCache sample config with your recommended settings for sustained cluster throughput.

                      Thanks again,

                      Brian.

                      • 8. Re: LockInterceptor.transactions - why a synch List?
                        belaban

                        It is on the roadmap :-)

                        I know, excuses left and right ...

                        (overworked) Bela