Derby
  1. Derby
  2. DERBY-5623

Loosen up synchronization in FileMonitor

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.8.2.2
    • Fix Version/s: 10.9.1.0
    • Component/s: Services
    • Labels:
      None
    • Bug behavior facts:
      Performance

      Description

      When opening a large number of databases in parallel, many threads get stuck for a long time in FileMonitor, as reported in this thread on derby-user:
      http://mail-archives.apache.org/mod_mbox/db-derby-user/201202.mbox/%3CCAEQ8c00vx0yFrwWyjm%3Dt_Yd%2BuuKGOL%3DSgpYfQuYghOOPgdN8-w%40mail.gmail.com%3E

      The synchronization in FileMonitor is only needed because the monitor instance is also used as a shared PrivilegedExceptionAction instance. Since all databases share one FileMonitor instance, threads that access any of these methods are serialized. If each method created its own PrivilegedAction or PrivilegedExceptionAction instance instead of using the monitor's run() method, these methods wouldn't need synchronization.

      1. d5623-1.diff
        8 kB
        Knut Anders Hatlen
      2. derby-diff.txt
        9 kB
        David Sitsky

        Activity

        Hide
        David Sitsky added a comment -

        Patch file to remove synchronisation in FileMonitor.

        Show
        David Sitsky added a comment - Patch file to remove synchronisation in FileMonitor.
        Hide
        Knut Anders Hatlen added a comment -

        Thanks for the patch, David. I had played with a similar patch myself (see the attached file, d5623-1.diff). My patch differs a little from your patch:

        • It preserves the synchronization in getDaemonThread(). Without synchronization in that method, I saw occasional IllegalThreadStateExceptions when booting and shutting down databases concurrently in many threads.
        • It uses PrivilegedAction instead of PrivilegedExceptionAction for the actions that don't throw checked exceptions, which made the exception handling simpler.

        All the regression tests passed with this patch.

        I also ran an application that booted 1000 databases in 20 threads on a machine with 8 cores, and the patch seemed to speed it up by 3-4%. It was primarily the removal of synchronization in getJVMProperty() that contributed to the speed-up.

        Although the speed-up is modest (at least in my environment) the patches remove more lines of code than they add, which is a good thing in itself.

        Show
        Knut Anders Hatlen added a comment - Thanks for the patch, David. I had played with a similar patch myself (see the attached file, d5623-1.diff). My patch differs a little from your patch: It preserves the synchronization in getDaemonThread(). Without synchronization in that method, I saw occasional IllegalThreadStateExceptions when booting and shutting down databases concurrently in many threads. It uses PrivilegedAction instead of PrivilegedExceptionAction for the actions that don't throw checked exceptions, which made the exception handling simpler. All the regression tests passed with this patch. I also ran an application that booted 1000 databases in 20 threads on a machine with 8 cores, and the patch seemed to speed it up by 3-4%. It was primarily the removal of synchronization in getJVMProperty() that contributed to the speed-up. Although the speed-up is modest (at least in my environment) the patches remove more lines of code than they add, which is a good thing in itself.
        Hide
        David Sitsky added a comment -

        Great - sounds good.

        Show
        David Sitsky added a comment - Great - sounds good.
        Hide
        Knut Anders Hatlen added a comment -

        I committed the patch with revision 1292724. Marking the issue as resolved.

        Show
        Knut Anders Hatlen added a comment - I committed the patch with revision 1292724. Marking the issue as resolved.

          People

          • Assignee:
            Knut Anders Hatlen
            Reporter:
            Knut Anders Hatlen
          • Votes:
            1 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development