Skip to content

Commit 0d39d52

Browse files
committed
Optimize Class#subclasses
There were several issues with the old implementation: * Iterating by creating a Set and an Iterator adds significant allocation overhead. Better to use Map#forEach. * ConcurrentWeakHashMap is a very old implementation that has several inefficiencies as well as no custom implementation of Map#forEach (it falls back on the default impl which creates a Set and Iterator). Instead we use a synchronized WeakHashMap. * The initial array was created with a default size of 4 elements. For many classes this will be insufficient, leading to excessive array allocation and copying as it grows. Instead we use the current class's "subclasses" size as an initial estimate. * When no subclasses appear to be present, bail out early with an empty array that does minimal allocation. Performance is significantly better for zero, small, and large subclass lists. 1M * Object.subclasses, with 83 elements: Before: 2.020000 0.040000 2.060000 ( 1.823539) 1.980000 0.030000 2.010000 ( 1.737808) 1.830000 0.010000 1.840000 ( 1.738772) 1.730000 0.020000 1.750000 ( 1.740675) 1.720000 0.000000 1.720000 ( 1.707344) After: 1.340000 0.020000 1.360000 ( 1.034745) 0.930000 0.000000 0.930000 ( 0.918107) 0.920000 0.010000 0.930000 ( 0.914828) 0.920000 0.000000 0.920000 ( 0.922137) 0.920000 0.000000 0.920000 ( 0.915440) 10M * Numeric.subclasses, with 4 elements: Before: 0.930000 0.030000 0.960000 ( 0.789997) 0.640000 0.010000 0.650000 ( 0.621404) 0.620000 0.010000 0.630000 ( 0.614404) 0.630000 0.000000 0.630000 ( 0.629492) 0.630000 0.010000 0.640000 ( 0.608538) After: 0.720000 0.010000 0.730000 ( 0.559470) 0.460000 0.000000 0.460000 ( 0.454176) 0.510000 0.000000 0.510000 ( 0.429875) 0.430000 0.010000 0.440000 ( 0.434487) 0.430000 0.000000 0.430000 ( 0.427971) Fixes jruby#8457
1 parent 8b022a9 commit 0d39d52

File tree

1 file changed

+19
-12
lines changed

1 file changed

+19
-12
lines changed

core/src/main/java/org/jruby/RubyClass.java

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@
9393
import org.jruby.util.Loader;
9494
import org.jruby.util.OneShotClassLoader;
9595
import org.jruby.util.StringSupport;
96-
import org.jruby.util.collections.ConcurrentWeakHashMap;
96+
import org.jruby.util.WeakIdentityHashMap;
9797
import org.jruby.util.log.Logger;
9898
import org.jruby.util.log.LoggerFactory;
9999
import org.objectweb.asm.AnnotationVisitor;
@@ -1019,7 +1019,14 @@ protected void setModuleSuperClass(RubyClass superClass) {
10191019

10201020
@JRubyMethod
10211021
public IRubyObject subclasses(ThreadContext context) {
1022-
RubyArray<RubyClass> subs = RubyArray.newArray(context.runtime);
1022+
Map<RubyClass, Object> subclasses = this.subclasses;
1023+
int sizeEstimate = subclasses == null ? 0 : subclasses.size();
1024+
1025+
if (sizeEstimate == 0) {
1026+
return RubyArray.newEmptyArray(context.runtime);
1027+
}
1028+
1029+
RubyArray<RubyClass> subs = RubyArray.newArray(context.runtime, sizeEstimate);
10231030

10241031
concreteSubclasses(subs);
10251032

@@ -1081,18 +1088,18 @@ private void subclassesInner(Collection<RubyClass> mine, boolean includeDescenda
10811088
}
10821089
}
10831090

1084-
private void concreteSubclasses(Collection<RubyClass> subs) {
1091+
private void concreteSubclasses(RubyArray<RubyClass> subs) {
10851092
Map<RubyClass, Object> subclasses = this.subclasses;
10861093
if (subclasses != null) {
1087-
Set<RubyClass> keys = subclasses.keySet();
1088-
for (RubyClass klass: keys) {
1089-
if (klass.isSingleton()) continue;
1090-
if (klass.isIncluded() || klass.isPrepended()) {
1091-
klass.concreteSubclasses(subs);
1092-
continue;
1094+
subclasses.forEach((klass, $) -> {
1095+
if (!klass.isSingleton()) {
1096+
if (klass.isIncluded() || klass.isPrepended()) {
1097+
klass.concreteSubclasses(subs);
1098+
} else {
1099+
subs.append(klass);
1100+
}
10931101
}
1094-
subs.add(klass);
1095-
}
1102+
});
10961103
}
10971104
}
10981105

@@ -1112,7 +1119,7 @@ public void addSubclass(RubyClass subclass) {
11121119
synchronized (this) {
11131120
subclasses = this.subclasses;
11141121
if (subclasses == null) {
1115-
this.subclasses = subclasses = new ConcurrentWeakHashMap<>(4, 0.75f, 1);
1122+
this.subclasses = subclasses = Collections.synchronizedMap(new WeakHashMap<>(4));
11161123
}
11171124
}
11181125
}

0 commit comments

Comments
 (0)