Skip to content

Commit 83ebf81

Browse files
richhickeyrichhickey
committed
reduce lock contention in MultiFn
1 parent 412a51d commit 83ebf81

File tree

2 files changed

+120
-52
lines changed

2 files changed

+120
-52
lines changed

‎clojure.iml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@
1717
</content>
1818
<orderEntry type="inheritedJdk" />
1919
<orderEntry type="sourceFolder" forTests="false" />
20+
<orderEntry type="library" scope="PROVIDED" name="Maven: org.codehaus.jsr166-mirror:jsr166y:1.7.0" level="project" />
21+
<orderEntry type="library" scope="TEST" name="Maven: org.clojure:test.generative:0.1.9" level="project" />
22+
<orderEntry type="library" scope="TEST" name="Maven: org.clojure:tools.namespace:0.1.1" level="project" />
23+
<orderEntry type="library" scope="TEST" name="Maven: org.clojure:java.classpath:0.1.1" level="project" />
2024
</component>
2125
</module>
2226

‎src/jvm/clojure/lang/MultiFn.java

Lines changed: 116 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -13,23 +13,26 @@
1313
package clojure.lang;
1414

1515
import java.util.Map;
16+
import java.util.concurrent.locks.ReentrantReadWriteLock;
1617

1718
public class MultiFn extends AFn{
1819
final public IFn dispatchFn;
1920
final public Object defaultDispatchVal;
2021
final public IRef hierarchy;
2122
final String name;
22-
IPersistentMap methodTable;
23-
IPersistentMap preferTable;
24-
IPersistentMap methodCache;
25-
Object cachedHierarchy;
23+
final ReentrantReadWriteLock rw;
24+
volatile IPersistentMap methodTable;
25+
volatile IPersistentMap preferTable;
26+
volatile IPersistentMap methodCache;
27+
volatile Object cachedHierarchy;
2628

2729
static final Var assoc = RT.var("clojure.core", "assoc");
2830
static final Var dissoc = RT.var("clojure.core", "dissoc");
2931
static final Var isa = RT.var("clojure.core", "isa?");
3032
static final Var parents = RT.var("clojure.core", "parents");
3133

3234
public MultiFn(String name, IFn dispatchFn, Object defaultDispatchVal, IRef hierarchy) {
35+
this.rw = new ReentrantReadWriteLock();
3336
this.name = name;
3437
this.dispatchFn = dispatchFn;
3538
this.defaultDispatchVal = defaultDispatchVal;
@@ -40,35 +43,63 @@ public MultiFn(String name, IFn dispatchFn, Object defaultDispatchVal, IRef hier
4043
cachedHierarchy = null;
4144
}
4245

43-
synchronized public MultiFn reset(){
44-
methodTable = methodCache = preferTable = PersistentHashMap.EMPTY;
45-
cachedHierarchy = null;
46-
return this;
46+
public MultiFn reset(){
47+
rw.writeLock().lock();
48+
try{
49+
methodTable = methodCache = preferTable = PersistentHashMap.EMPTY;
50+
cachedHierarchy = null;
51+
return this;
52+
}
53+
finally {
54+
rw.writeLock().unlock();
55+
}
4756
}
4857

49-
synchronized public MultiFn addMethod(Object dispatchVal, IFn method) {
50-
methodTable = getMethodTable().assoc(dispatchVal, method);
51-
resetCache();
52-
return this;
58+
public MultiFn addMethod(Object dispatchVal, IFn method) {
59+
rw.writeLock().lock();
60+
try{
61+
methodTable = getMethodTable().assoc(dispatchVal, method);
62+
resetCache();
63+
return this;
64+
}
65+
finally {
66+
rw.writeLock().unlock();
67+
}
5368
}
5469

55-
synchronized public MultiFn removeMethod(Object dispatchVal) {
56-
methodTable = getMethodTable().without(dispatchVal);
57-
resetCache();
58-
return this;
70+
public MultiFn removeMethod(Object dispatchVal) {
71+
rw.writeLock().lock();
72+
try
73+
{
74+
methodTable = getMethodTable().without(dispatchVal);
75+
resetCache();
76+
return this;
77+
}
78+
finally
79+
{
80+
rw.writeLock().unlock();
81+
}
5982
}
6083

61-
synchronized public MultiFn preferMethod(Object dispatchValX, Object dispatchValY) {
62-
if(prefers(dispatchValY, dispatchValX))
63-
throw new IllegalStateException(
64-
String.format("Preference conflict in multimethod '%s': %s is already preferred to %s",
65-
name, dispatchValY, dispatchValX));
66-
preferTable = getPreferTable().assoc(dispatchValX, RT.conj((IPersistentCollection) RT.get(getPreferTable(),
67-
dispatchValX,
68-
PersistentHashSet.EMPTY),
69-
dispatchValY));
70-
resetCache();
71-
return this;
84+
public MultiFn preferMethod(Object dispatchValX, Object dispatchValY) {
85+
rw.writeLock().lock();
86+
try
87+
{
88+
if(prefers(dispatchValY, dispatchValX))
89+
throw new IllegalStateException(
90+
String.format("Preference conflict in multimethod '%s': %s is already preferred to %s",
91+
name, dispatchValY, dispatchValX));
92+
preferTable = getPreferTable().assoc(dispatchValX, RT.conj((IPersistentCollection) RT.get(getPreferTable(),
93+
dispatchValX,
94+
PersistentHashSet.EMPTY),
95+
dispatchValY));
96+
resetCache();
97+
return this;
98+
}
99+
finally
100+
{
101+
rw.writeLock().unlock();
102+
}
72103
}
73104

74105
private boolean prefers(Object x, Object y) {
@@ -97,18 +128,26 @@ private boolean dominates(Object x, Object y) {
97128
}
98129

99130
private IPersistentMap resetCache() {
100-
methodCache = getMethodTable();
101-
cachedHierarchy = hierarchy.deref();
102-
return methodCache;
131+
rw.writeLock().lock();
132+
try
133+
{
134+
methodCache = getMethodTable();
135+
cachedHierarchy = hierarchy.deref();
136+
return methodCache;
137+
}
138+
finally
139+
{
140+
rw.writeLock().unlock();
141+
}
103142
}
104143

105-
synchronized public IFn getMethod(Object dispatchVal) {
144+
public IFn getMethod(Object dispatchVal) {
106145
if(cachedHierarchy != hierarchy.deref())
107146
resetCache();
108147
IFn targetFn = (IFn) methodCache.valAt(dispatchVal);
109148
if(targetFn != null)
110149
return targetFn;
111-
targetFn = findAndCacheBestMethod(dispatchVal);
150+
targetFn = findAndCacheBestMethod(dispatchVal);
112151
if(targetFn != null)
113152
return targetFn;
114153
targetFn = (IFn) getMethodTable().valAt(defaultDispatchVal);
@@ -124,34 +163,59 @@ private IFn getFn(Object dispatchVal) {
124163
}
125164

126165
private IFn findAndCacheBestMethod(Object dispatchVal) {
127-
Map.Entry bestEntry = null;
128-
for(Object o : getMethodTable())
166+
rw.readLock().lock();
167+
Map.Entry bestEntry;
168+
IPersistentMap mt = methodTable;
169+
IPersistentMap pt = preferTable;
170+
Object ch = cachedHierarchy;
171+
try
129172
{
130-
Map.Entry e = (Map.Entry) o;
131-
if(isA(dispatchVal, e.getKey()))
173+
bestEntry = null;
174+
for(Object o : getMethodTable())
132175
{
133-
if(bestEntry == null || dominates(e.getKey(), bestEntry.getKey()))
134-
bestEntry = e;
135-
if(!dominates(bestEntry.getKey(), e.getKey()))
136-
throw new IllegalArgumentException(
137-
String.format(
138-
"Multiple methods in multimethod '%s' match dispatch value: %s -> %s and %s, and neither is preferred",
139-
name, dispatchVal, e.getKey(), bestEntry.getKey()));
176+
Map.Entry e = (Map.Entry) o;
177+
if(isA(dispatchVal, e.getKey()))
178+
{
179+
if(bestEntry == null || dominates(e.getKey(), bestEntry.getKey()))
180+
bestEntry = e;
181+
if(!dominates(bestEntry.getKey(), e.getKey()))
182+
throw new IllegalArgumentException(
183+
String.format(
184+
"Multiple methods in multimethod '%s' match dispatch value: %s -> %s and %s, and neither is preferred",
185+
name, dispatchVal, e.getKey(), bestEntry.getKey()));
186+
}
140187
}
188+
if(bestEntry == null)
189+
return null;
141190
}
142-
if(bestEntry == null)
143-
return null;
191+
finally
192+
{
193+
rw.readLock().unlock();
194+
}
195+
196+
144197
//ensure basis has stayed stable throughout, else redo
145-
if(cachedHierarchy == hierarchy.deref())
198+
rw.writeLock().lock();
199+
try
146200
{
147-
//place in cache
148-
methodCache = methodCache.assoc(dispatchVal, bestEntry.getValue());
149-
return (IFn) bestEntry.getValue();
201+
if( mt == methodTable &&
202+
pt == preferTable &&
203+
ch == cachedHierarchy &&
204+
cachedHierarchy == hierarchy.deref())
205+
{
206+
//place in cache
207+
methodCache = methodCache.assoc(dispatchVal, bestEntry.getValue());
208+
return (IFn) bestEntry.getValue();
209+
}
210+
else
211+
{
212+
resetCache();
213+
return findAndCacheBestMethod(dispatchVal);
214+
}
150215
}
151-
else
216+
finally
152217
{
153-
resetCache();
154-
return findAndCacheBestMethod(dispatchVal);
218+
rw.writeLock().unlock();
155219
}
156220
}
157221

0 commit comments

Comments
 (0)