castor
  1. castor
  2. CASTOR-3085 Refactor the ObjectLock class
  3. CASTOR-3122

Use ReetrantLock and Condition to instead of "synchronized, wait, notifyAll".

    Details

    • Type: Sub-task Sub-task
    • Status: Resolved Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.3.2
    • Fix Version/s: 1.3.3rc1
    • Component/s: JDO
    • Labels:
      None
    • Number of attachments :
      3

      Description

      In current implementation, synchronized, wait and notifyAll make the code difficult to understand. I'll use ReetrantLock and Condition to make them clear.

      1. patch-C3122-20110721.txt
        22 kB
        Wensheng Dou
      2. patch-C3122-20110722.txt
        20 kB
        Ralf Joachim
      3. patch-C3122-20110722.txt
        17 kB
        Wensheng Dou

        Activity

        Hide
        Wensheng Dou added a comment -

        In this patch, I use ReentrantReadWriteLock and Condition to refactor the implementation.

        Show
        Wensheng Dou added a comment - In this patch, I use ReentrantReadWriteLock and Condition to refactor the implementation.
        Hide
        Ralf Joachim added a comment -

        I wonder why you lock all methods in this new patch. Doesn't this produce additional overhead? E.g. calls to enter() and leave() don't need to be locked. We even don't need to use an AtomicInteger as they are only called inside of synchronized blocks in TypeInfo.

        Show
        Ralf Joachim added a comment - I wonder why you lock all methods in this new patch. Doesn't this produce additional overhead? E.g. calls to enter() and leave() don't need to be locked. We even don't need to use an AtomicInteger as they are only called inside of synchronized blocks in TypeInfo.
        Hide
        Wensheng Dou added a comment -

        yes, you are right, some locking are not necessary now, they produce some additional overhead. Why I lock all methods, I have some consideration as follows:
        (1) I think all the fields in the ObjectLock should be protected by only one lock (not two locks), this will make the code more understandable.
        (2) I'm considering to remove the synchronized(locks) in the TypeInfo, and this may be a bottleneck. This will need some other methods in ObjectLock protected by the lock.

        I'm not sure about (2) now. I think you are right. I should just protected the methods as same as the 'synchronized' now. So I create a new patch for this.

        Show
        Wensheng Dou added a comment - yes, you are right, some locking are not necessary now, they produce some additional overhead. Why I lock all methods, I have some consideration as follows: (1) I think all the fields in the ObjectLock should be protected by only one lock (not two locks), this will make the code more understandable. (2) I'm considering to remove the synchronized(locks) in the TypeInfo, and this may be a bottleneck. This will need some other methods in ObjectLock protected by the lock. I'm not sure about (2) now. I think you are right. I should just protected the methods as same as the 'synchronized' now. So I create a new patch for this.
        Hide
        Ralf Joachim added a comment -

        Final patch

        Show
        Ralf Joachim added a comment - Final patch

          People

          • Assignee:
            Wensheng Dou
            Reporter:
            Wensheng Dou
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: