• Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

Lock design

 
Adrian Yan
Ranch Hand
Posts: 688
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
While I'm refactoring my design on locks, I figured I should throw my design and reasons here and discuss about it.
My design is using a HastSet object (HashSet is thread-safe by itself), let's call it dbLocks, it's used to hold all records that have been locked.

------------------

[This message has been edited by Adrian Yan (edited April 04, 2001).]
 
Matt DeLacey
Ranch Hand
Posts: 318
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
That's basically what I'm doing...one thing though...if HashSet is threadsafe, do you need to synchronize on it? Isn't that the point of making a collection threadsafe, so you won't have to? I synchronixed on my data set too, but used a vector, so I'm not saying that's so, just curious.
With Respect,
Matt Delacey
 
Aron J. Skantz
Greenhorn
Posts: 26
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Adrian,
Your design is nearly identical to mine, except that I also have a -1 logic to handle locking of all records.
If I had to change something in your code, a
while(dbLocks.contains(...))
eliminates the need of that extra boolean :-)
j/k
And yes, it is recommended to use notifyAll instead of notify, probably dont make that big difference in this case though..
Regards
Aron
 
Adrian Yan
Ranch Hand
Posts: 688
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Huuuu???
gautam shah, I'm not sure if I understand your comments.
My design is simply my design, nothing more. I'm not trying to make any statements here, I'm not saying my design is better than everybody else. As far as my comments on my code, there are simply my reasons.
Can you please elaborate on your comments abit? I'm kind of confuse here.
 
Matt DeLacey
Ranch Hand
Posts: 318
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
guatum shah,
the reason he calls dbLocks.wait() and dbLocks.notifyAll()
is because you have to or else you'll get some kind of wacky MonitorStateException or some such. You have to call wait and notifyAll on the monitor.
 
Aron J. Skantz
Greenhorn
Posts: 26
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Actually Gautam...
I really don't understand what you are talking about, but I will try to explain anyway.
Calling only this.wait() in Adrians (and my) case as you suggest will not work since it results in a IllegalMonitorStateException, the reason for this is simple:
synchronized(dbLocks) obtains the monitor for the object dbLocks, and thus you MUST call wait on that object. Calling this.wait() here fails since you don't own that monitor. The same goes for notifyAll().
So, again.. I don't know what you are talking about, what convention?

Regards,
Aron
 
Aron J. Skantz
Greenhorn
Posts: 26
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Im very close to not answering this since I don't "know the proper use of wait and notify."
Sigh, I will try to explain again, slowly this time:
Do you agree that you have to own the monitor of an object to be able to do a wait(), Im sure you do.
In Adrians example you had to call dbLocks.wait() since we synchronized on dbLocks, calling only this.wait() will fail.
In your example you are synchronizing on java, ok then we have the monitor for the java object, agree?
Your wait and notify, are the same as this.wait() and this.notify(), then how can this work for you if you have the monitor on java? Simple, they are INSIDE THE JAVARANCH CLASS. Thus, they will do wait and notify ON THE JAVA object since its an instance of the Javaranch class.
One more time: calling this.wait() inside the Javaranch object java, is equal to calling java.wait() in the run method.
So, the examplecode you provided only shows that in Adrians code you have to do dbLocks.wait() so I dont understand why you posted it to prove your point.

Regards,
Aron
[This message has been edited by Aron J. Skantz (edited April 05, 2001).]
 
Douglas Kent
Ranch Hand
Posts: 171
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I'm synchronizing on the Data methods lock and unlock. Heller et al state that this is the preferred technique rather than synchronizing on objects. Any thoughts?
Also, don't see the advantage of using notifyall()...the awakened threads will still have to compete for access to the thread anyway...why not just let the JVM pick one and let it go?
 
Douglas Kent
Ranch Hand
Posts: 171
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Pardon me, I meant to say "access to the synchronized object" vs "thread".
 
Matt DeLacey
Ranch Hand
Posts: 318
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Douglas,
If I understand you correctly, you propose to make lock and unlock synchronized? I think this would be a mistake. It would work, but performance would suffer. Suppose one client wants access to record #1 and another client want access to record #2. Why should one have to wait on the other? They want different records so there is no need for them to wait for each other. Snychronizing on the method would make clients who should not b competing against each other have to compete.
With Respect,
Matt DeLacey
 
Matt Cannata
Greenhorn
Posts: 23
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
The one problem with making the lock() and unlock() methods synchronized (or synchronizing on this) is that all the access methods in the Data class are sychronized. Therefore, no other methods will be able to execute while you are locking or unlocking, which affects performance.
It also causes wasteful cycles by waking up threads unnecessairly. Example: Clients 1-5 are waiting or record 5. Client 6 releases lock or record 1 and calls notifyAll. Clients 1-5 get notified, since they were all waiting on the same object (Data) and all of them try to get record that is still unavailable.
I don't know of synchronizing on the Data object will affect your score, but as you can see it will affect performance.
 
Rahul Rathore
Ranch Hand
Posts: 324
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Matt DeLacey
With respect your reasoning is incorrect. If you see your design, synchronizing on the dbLocks object (or whatever) will have the same effect. If you walk through your code, you will see that even in your design, a thread trying to lock rec #1 will have to wait while a thread is trying to lock rec #2 or vice versa, because all threads whatever their target record, are synchronizing on the SAME object (whether dbLocks or whatever). The only way to avoid this would be to sychronize on multiple objects- i.e. if there are 10 records then create 10 dummy lock object- one object for each record. Then make the threads trying to lock rec #1 synchronize on lock1, those trying to lock rec #2 synchronize on lock2 and so on ... This will make the design complicated.
But you would have been accurate if you had pointed out the advantage of your design is that threads trying to acquire record locks would not need to synchronize with threads doing read/write of the database.
But even though this advantage may be there, it does tend to complicate the design. I do tend to agree with Douglas that synchronizing methods (i.e. synchronizing on 'this') is better because it leads to a simpler, more readable, and maintainable design. Synchronizing on different objects requires one to be very very careful - to avoid designs which would be prone to deadlocks and nested lockouts.
Best Reg.
[This message has been edited by Rahul Rathore (edited April 05, 2001).]
 
Rahul Rathore
Ranch Hand
Posts: 324
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Matt Cannata
Originally posted by Matt Cannata:
It also causes wasteful cycles by waking up threads unnecessairly. Example: Clients 1-5 are waiting or record 5. Client 6 releases lock or record 1 and calls notifyAll. Clients 1-5 get notified, since they were all waiting on the same object (Data) and all of them try to get record that is still unavailable.

How do you ensure that if record 1 is released, notify is NOT received by Clients waiting for record 5 ??
Unless you have implemented some sophisticated design scheme, or adopted a different synchronizing object for each record, the above is not possible. If all lock-seeking threads are synchronizing on the SAME object irrespective of their target record, then you will have to notify them all. I think there is specific notification pattern in Doug Lea's Concurrent Programming- though I haven't read it yet. Are you using that?
 
Matt DeLacey
Ranch Hand
Posts: 318
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Rahul,
I believe that you are absolutely right. Thanks for pointing this out to me. Not sure how it escaped me. I was thinking of locking on a wrapped version of the record rather than on a collections class. I don't like at all the idea of two clients who want separate records having to wait on one another. I will explore my other idea now. Thanks again.
With Respect,
Matt DeLacey
 
Matt Cannata
Greenhorn
Posts: 23
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I adopted a scheme similar to the one you mentioned. Instead of keeping a collection of locked records, I keep a collection with size equal to the number of records. This collection references objects indicating whether or not the record is locked. Then, I synchronize on those objects. The only changes this method requires is the synchronization tag and a little bit of initialization/maintence in constructor/add/delete.
 
Dilip kumar
Ranch Hand
Posts: 360
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I'm having synchronized lock() and unlock() methods in a locking helper class. I'm calling these synchronized methods from the unsynchronized lock() and unlock() methods in the Data class. This way I'm avoiding more synchronized methods Data class. I feel this will improve the performace a little bit.
 
kamal kant
Greenhorn
Posts: 5
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Rahul & Matt
Are you talking about some following kind of lock , unlock procedures. This is just my try only and nothing else. because of i am an electrical engineer so it may be the case that this procedure might be stupid.
<pre><code>Hashtable lockTable;
public void lock(int i) {
Object o = lockTable.get(new Integer(i));
while(true) {
if (o==null){
lockTable.put(new Integer(i),new Object());
break;
} else {
synchronized (o) {
o.wait();
}
}
}
public void unlock(int i){
Object o = lockTable.get(new Integer(i));
synchronized (o) {
o.notify();
lockTable.remove(o);
}
}
</code></pre>

[This message has been edited by kamal kant (edited April 06, 2001).]
 
Matt Cannata
Greenhorn
Posts: 23
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
kamal,
Your prodedure is close, but it needs a little modification to work. The main problem is that your Object reference, o, never changes in the lock() method. You will need to check it in the loop, which will require you to increase the size of the synchronized block.
Depending on the exact code, I don't think it's necessary to have a mutable lock indicator (instead of calling new Object()), but it at least makes sense from an Object creation/gargage collection standpoint.

Finally, note that a HashTable is not needed. Indexes into a Vector work fine as a hash function to record number.
 
Andreas Ronge
Greenhorn
Posts: 9
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Dear Rahul & Matt
Two clients who want separate records do not have to wait on one another. They only have to wait if they want to execute the same synchronized code block at the same time. This is not a problem, just a minor performance issue (a client will quickly leave the small synchronized block).
If the first client receives the lock he will then exit the synchronized code block, that means that he will not block the next client who wants to lock a different record. And if the first client does not receive the lock he will wait and leave the
monitor, the next client who wants to lock a different record will therefore not have to wait for the first client.
Regards,
Andreas
Originally posted by Matt DeLacey:
Rahul,
I believe that you are absolutely right. Thanks for pointing this out to me. Not sure how it escaped me. I was thinking of locking on a wrapped version of the record rather than on a collections class. I don't like at all the idea of two clients who want separate records having to wait on one another. I will explore my other idea now. Thanks again.
With Respect,
Matt DeLacey

 
Matt Cannata
Greenhorn
Posts: 23
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Andreas,
You are correct about execution order of the synchronized code, but the point of my design is to correct un-necessary wake ups. If clients block on the same object no matter what record they request, then they will all be woken up when notifyAll is called, regardless of whether or not the record they requested is the one being released. In an enterprise system, this can get very costly as thousands of clients can be notified while their record is unavailable.
Matt
 
Matt DeLacey
Ranch Hand
Posts: 318
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Actually,
I'm pretty sure he was talking to me. I've gone back and forth
on this issue so many times. Thanks for bringing this to my attention. This is the last thing I need to take care of before submission, is making my understanding of this complete. I am going to run a lot of tests and figure out precisely what is going on. Thanks though for pointing this out to me.
With Respect,
Matt
 
Thomas Mathai
Greenhorn
Posts: 9
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
The issue that Matt brought up also means that if you are synchronizing on Vector or some such common object, you will need to call notifyAll instead of notify. If you call notify, you will only wake up one thread and this thread may be waiting on another record!
 
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic