Re: Cannot seem to lock HashMap
byoder@hotmail.com wrote:
Lew -
Sorry for the TABS (I copied code from Eclipse). I do agree that your
code may work, but I would be paying penalty of having to synchronize
(lock) for every call that uses "values".
It is necessary to synchronize every access to a shared resource if you want
to avoid memory model anomalies.
But I found solution that allows me to ONLY synchronize methods that
either depend on the map NOT getting modified, and methods that modify
the map. To do this I must synchronize all methods that can modify
the map, but all methods that don't need to synchronize the map (such
as get() don't depend on any locking) are not synchronized so there is
no locking and thus are faster.
This means that you risk having the get() return different values than were put().
Any methods that depend on the map data TO NOT CHANGE I explicitly
lock the container class. This is the best approach because I don't
have to pay penalty for locking (synchronizing) all of my methods -
The "penalty" is not so large, and certainly smaller than getting the wrong
results.
which is the solution I was looking for as speed is very important to
me in this code.
But correct results are not?
See example (two classes):
public class TestIndex {
public static void main(String[] args) {
TestContainer c = new TestContainer();
new Thread_1(c).start();
new Thread_2(c).start();
}
public static class Thread_1 extends Thread {
TestContainer c;
public Thread_1(TestContainer c) {
super("Thread_1");
this.setDaemon(false);
this.c = c;
}
public void run() {
Date start = new Date();
System.out.println("Thread_1 run...");
try {
for (int i=0; i<1000000; i++) {
Date key = new Date();
c.put(key,new Date());
c.get(key);
}
} catch (Exception e) {
e.printStackTrace();
}
Date end = new Date();
long diff = end.getTime() - start.getTime();
System.out.println("Thread_1 END: " + diff + " total time");
}
}
public static class Thread_2 extends Thread {
TestContainer c;
public Thread_2(TestContainer c) {
super("Thread_2");
this.setDaemon(false);
this.c = c;
}
public void run() {
Date start = new Date();
System.out.println("Thread_2 run...");
try {
for (int i=0; i<1000000; i++) {
c.clone();
}
} catch (Exception e) {
e.printStackTrace();
}
Date end = new Date();
long diff = end.getTime() - start.getTime();
System.out.println("Thread_2 END: " + diff + " total time");
}
}
}
public class TestContainer {
private HashMap<Date, Date> _values;
Apparently you overlooked the suggestion to declare this as a Map rather than
a HashMap.
public TestContainer() {
_values = new HashMap<Date, Date>();
}
public TestContainer(HashMap<Date, Date> v) {
_values = v;
}
public void put(Date key, Date value) {
_values.put(key, value);
This unsynchronized access will cause trouble.
}
public Date get(Date key) {
return _values.get(key);
This unsynchronized access will cause trouble.
}
public Object clone() {
HashMap<Date, Date> cValues = new HashMap<Date,
Date>(_values.size());
This unsynchronized access will cause trouble.
Declare the variable as the interface type, instantiate as the concrete type.
synchronized (this) {
cValues.putAll(_values);
Synchronizing on only one access to a protected resource but leaving the rest
unsynchronized causes bugs.
}
return new TestContainer(cValues);
}
}
Your code is subject to a host of ills because you aren't synchronizing your
access to a shared resource.
--
Lew