Skip to content

Commit d5a7c80

Browse files
authored
use ClassValue for caching class-based TypeInfo (mozilla#2255)
Rewrite TypeInfoFactory to provide 2(concurrent/weak) * 2(ClassValue/Map) = 4 factory choices Improve caching behavior to use ClassValue when available, and fall back on old environments (like Android < 34) when it is not, and add tests to ensure that.
1 parent 7f7314f commit d5a7c80

14 files changed

Lines changed: 327 additions & 194 deletions

File tree

.github/workflows/android.yml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ jobs:
1414
strategy:
1515
fail-fast: false
1616
matrix:
17-
api-level: [ 26, 28, 30, 33 ]
17+
# oldest supported API level: API 26 (Android 8)
18+
# current latest API level: API 34 (Android 14)
19+
api-level: [ 26, 28, 30, 34 ]
1820
name: Rhino Android Tests
1921
steps:
2022
- name: Enable KVM
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/*
2+
Basic test for several LiveConnect features
3+
*/
4+
5+
let l = new java.util.ArrayList()
6+
7+
// method
8+
assertEquals(0, l.size())
9+
// beaning (.isEmpty())
10+
assertTrue(l.empty)
11+
12+
// method overload
13+
l.add("0")
14+
l.add(0, "1")
15+
assertEquals(2, l.size())
16+
assertEquals("1", l.get(0) + '')
17+
18+
let arr = java.lang.reflect.Array.newInstance(java.lang.Byte, 10)
19+
20+
// field
21+
assertEquals(10, arr.length)
22+
23+
"success"

it-android/src/main/java/org/mozilla/javascript/android/TestCase.java

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,10 @@
2222
*
2323
* @author Roland Praml
2424
*/
25-
public class TestCase {
25+
public abstract class TestCase {
2626

27-
private final String name;
28-
private final Scriptable global;
29-
private final AssetManager assetManager;
27+
protected final String name;
28+
protected final Scriptable global;
3029

3130
private static final ContextFactory factory =
3231
new ContextFactory() {
@@ -48,34 +47,49 @@ protected Context makeContext() {
4847
}
4948
};
5049

51-
public TestCase(String name, Scriptable global, AssetManager assetManager) {
50+
public TestCase(String name, Scriptable global) {
5251
this.name = name;
5352
this.global = global;
54-
this.assetManager = assetManager;
5553
}
5654

5755
public String run() {
5856
Context cx = factory.enterContext();
59-
try (InputStream in = assetManager.open("tests/" + name);
60-
Reader rdr = new InputStreamReader(in, StandardCharsets.UTF_8)) {
61-
57+
try {
6258
Scriptable scope = cx.newObject(global);
6359
scope.setPrototype(global);
6460
scope.setParentScope(null);
65-
Object result = cx.evaluateReader(scope, rdr, name, 1, null);
66-
return ScriptRuntime.toString(result);
67-
} catch (IOException e) {
68-
throw new UncheckedIOException(e);
61+
return ScriptRuntime.toString(runTest(cx, scope));
6962
} finally {
7063
Context.exit();
7164
}
7265
}
7366

67+
protected abstract Object runTest(Context cx, Scriptable scope);
68+
7469
@Override
7570
public String toString() {
7671
return name;
7772
}
7873

74+
public static class AssetScript extends TestCase {
75+
protected final AssetManager assetManager;
76+
77+
public AssetScript(String name, Scriptable global, AssetManager assetManager) {
78+
super(name, global);
79+
this.assetManager = assetManager;
80+
}
81+
82+
@Override
83+
protected Object runTest(Context cx, Scriptable scope) {
84+
try (InputStream in = assetManager.open("tests/" + name);
85+
Reader rdr = new InputStreamReader(in, StandardCharsets.UTF_8)) {
86+
return cx.evaluateReader(scope, rdr, name, 1, null);
87+
} catch (IOException e) {
88+
throw new UncheckedIOException(e);
89+
}
90+
}
91+
}
92+
7993
public static List<TestCase> getTestCases(android.content.Context context) throws IOException {
8094

8195
AssetManager assetManager = context.getAssets();
@@ -95,9 +109,10 @@ public static List<TestCase> getTestCases(android.content.Context context) throw
95109
List<TestCase> tests = new ArrayList<>();
96110
if (files != null) {
97111
for (String file : files) {
98-
tests.add(new TestCase(file, global, assetManager));
112+
tests.add(new TestCase.AssetScript(file, global, assetManager));
99113
}
100114
}
115+
tests.add(new TypeInfoFactoryTestCase("TypeInfoFactory", global));
101116
return tests;
102117
}
103118
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package org.mozilla.javascript.android;
2+
3+
import org.mozilla.javascript.Context;
4+
import org.mozilla.javascript.Scriptable;
5+
import org.mozilla.javascript.lc.type.TypeInfoFactory;
6+
import org.mozilla.javascript.lc.type.impl.factory.ClassValueCacheFactory;
7+
8+
/**
9+
* @author ZZZank
10+
*/
11+
public class TypeInfoFactoryTestCase extends TestCase {
12+
public TypeInfoFactoryTestCase(String name, Scriptable global) {
13+
super(name, global);
14+
}
15+
16+
@Override
17+
protected Object runTest(Context cx, Scriptable scope) {
18+
TypeInfoFactory typeInfoFactory = TypeInfoFactory.get(scope);
19+
int apiLevel = android.os.Build.VERSION.SDK_INT;
20+
if (apiLevel >= 34 != typeInfoFactory instanceof ClassValueCacheFactory) {
21+
throw new IllegalStateException(
22+
String.format(
23+
"ClassValueCacheFactory should be used for modern Android, but got %s on API level %s",
24+
typeInfoFactory, apiLevel));
25+
}
26+
return "success";
27+
}
28+
}

rhino/src/main/java/org/mozilla/javascript/ScriptRuntime.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@
2525
import org.mozilla.javascript.ast.FunctionNode;
2626
import org.mozilla.javascript.dtoa.DoubleFormatter;
2727
import org.mozilla.javascript.lc.type.TypeInfo;
28-
import org.mozilla.javascript.lc.type.impl.factory.ConcurrentFactory;
28+
import org.mozilla.javascript.lc.type.impl.factory.ClassValueCacheFactory;
29+
import org.mozilla.javascript.lc.type.impl.factory.LegacyCacheFactory;
2930
import org.mozilla.javascript.typedarrays.NativeArrayBuffer;
3031
import org.mozilla.javascript.typedarrays.NativeBigInt64Array;
3132
import org.mozilla.javascript.typedarrays.NativeBigUint64Array;
@@ -199,7 +200,11 @@ public static ScriptableObject initSafeStandardObjects(
199200

200201
scope.associateValue(LIBRARY_SCOPE_KEY, scope);
201202
new ClassCache().associate(scope);
202-
new ConcurrentFactory().associate(scope);
203+
var typeFactory =
204+
(androidApi >= 34 || androidApi < 0)
205+
? new ClassValueCacheFactory.Concurrent()
206+
: new LegacyCacheFactory.Concurrent();
207+
typeFactory.associate(scope);
203208

204209
LambdaConstructor function = BaseFunction.init(cx, scope, sealed);
205210
JSFunction obj = NativeObject.init(cx, scope, sealed);

rhino/src/main/java/org/mozilla/javascript/lc/type/TypeInfoFactory.java

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@
2323
import java.util.function.Supplier;
2424
import org.mozilla.javascript.Scriptable;
2525
import org.mozilla.javascript.ScriptableObject;
26-
import org.mozilla.javascript.lc.type.impl.factory.WeakReferenceFactory;
26+
import org.mozilla.javascript.lc.type.impl.factory.ClassValueCacheFactory;
27+
import org.mozilla.javascript.lc.type.impl.factory.LegacyCacheFactory;
2728

2829
/**
2930
* Factory for {@link TypeInfo}
@@ -54,12 +55,7 @@ public interface TypeInfoFactory extends Serializable {
5455
* <p>For actions with scope available, the TypeInfoFactory can be obtained via {@link
5556
* #get(Scriptable)}.
5657
*/
57-
TypeInfoFactory GLOBAL =
58-
new WeakReferenceFactory() {
59-
private Object readResolve() {
60-
return GLOBAL;
61-
}
62-
};
58+
TypeInfoFactory GLOBAL = createGlobalFactory();
6359

6460
TypeInfo[] EMPTY_ARRAY = new TypeInfo[0];
6561

@@ -392,4 +388,35 @@ static TypeInfoFactory getOrElse(Scriptable scope, TypeInfoFactory fallback) {
392388
}
393389
return got;
394390
}
391+
392+
/**
393+
* the {@code androidApi} field from {@link org.mozilla.javascript.ScriptRuntime} is not
394+
* accessible here, so we detect Android version manually
395+
*/
396+
private static TypeInfoFactory createGlobalFactory() {
397+
int androidAPI;
398+
try {
399+
androidAPI = Class.forName("android.os.Build$VERSION").getField("SDK_INT").getInt(null);
400+
} catch (Exception e) {
401+
if ("Dalvik".equals(System.getProperty("java.vm.name"))) {
402+
androidAPI = 1;
403+
} else {
404+
androidAPI = -1;
405+
}
406+
}
407+
408+
if (androidAPI >= 34 || androidAPI < 0) {
409+
// modern Android or not Android
410+
return new ClassValueCacheFactory.WeakReference() {
411+
private Object readResolve() {
412+
return GLOBAL;
413+
}
414+
};
415+
}
416+
return new LegacyCacheFactory.WeakReference() {
417+
private Object readResolve() {
418+
return GLOBAL;
419+
}
420+
};
421+
}
395422
}
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
package org.mozilla.javascript.lc.type.impl.factory;
2+
3+
import java.io.IOException;
4+
import java.io.ObjectInputStream;
5+
import java.io.ObjectOutputStream;
6+
import java.lang.reflect.TypeVariable;
7+
import java.util.Map;
8+
import org.mozilla.javascript.lc.type.TypeInfo;
9+
import org.mozilla.javascript.lc.type.TypeInfoFactory;
10+
import org.mozilla.javascript.lc.type.VariableTypeInfo;
11+
import org.mozilla.javascript.lc.type.impl.VariableTypeInfoImpl;
12+
13+
/**
14+
* {@link TypeInfoFactory} implementation with cache. The exact characteristic if the factory
15+
* depends on the characteristic of map backend created via {@link #initCache()}
16+
*
17+
* <p>This factory is serializable, but none of its cached objects will be serialized.
18+
*
19+
* @author ZZZank
20+
*/
21+
public abstract class AbstractCacheFactory implements FactoryBase, CachedFactory {
22+
private static final long serialVersionUID = 4533445095188189419L;
23+
24+
private transient Map<TypeVariable<?>, VariableTypeInfoImpl> variableCache;
25+
private transient Map<Class<?>, Map<VariableTypeInfo, TypeInfo>> consolidationMappingCache;
26+
27+
public AbstractCacheFactory() {
28+
initCache();
29+
}
30+
31+
protected void initCache() {
32+
variableCache = createCache();
33+
consolidationMappingCache = createCache();
34+
}
35+
36+
@Override
37+
public TypeInfo create(TypeVariable<?> typeVariable) {
38+
return variableCache.computeIfAbsent(
39+
typeVariable, raw -> new VariableTypeInfoImpl(raw, this));
40+
}
41+
42+
@Override
43+
public Map<VariableTypeInfo, TypeInfo> getConsolidationMapping(Class<?> from) {
44+
if (from == null || from == Object.class || from.isPrimitive()) {
45+
return Map.of();
46+
}
47+
// no computeIfAbsent because `computeTypeReplacement(...)` will recursively call this
48+
// method again
49+
var got = consolidationMappingCache.get(from);
50+
if (got == null) {
51+
got = computeConsolidationMapping(from);
52+
consolidationMappingCache.put(from, got);
53+
}
54+
return got;
55+
}
56+
57+
private void writeObject(ObjectOutputStream out) throws IOException {
58+
out.defaultWriteObject();
59+
}
60+
61+
private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException {
62+
in.defaultReadObject();
63+
initCache();
64+
}
65+
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
package org.mozilla.javascript.lc.type.impl.factory;
2+
3+
import java.util.Collections;
4+
import java.util.Map;
5+
import java.util.WeakHashMap;
6+
import java.util.concurrent.ConcurrentHashMap;
7+
import org.mozilla.javascript.lc.type.TypeInfoFactory;
8+
9+
/**
10+
* {@link Concurrent}: provides strong reference, thread-safe, performant cache
11+
*
12+
* <p>{@link WeakReference}: provides weak reference, thread-safe, less performant cache
13+
*
14+
* <p>{@link ClassValueCacheFactory} uses {@link ClassValue}, which provides weak reference,
15+
* thread-safe, most performant cache. But it's not available on some Android devices, and should
16+
* not be created once per instance
17+
*
18+
* @author ZZZank
19+
*/
20+
interface CachedFactory extends TypeInfoFactory {
21+
<K, V> Map<K, V> createCache();
22+
23+
interface Concurrent extends CachedFactory {
24+
25+
@Override
26+
default <K, V> Map<K, V> createCache() {
27+
return new ConcurrentHashMap<>();
28+
}
29+
}
30+
31+
interface WeakReference extends CachedFactory {
32+
33+
@Override
34+
default <K, V> Map<K, V> createCache() {
35+
return Collections.synchronizedMap(new WeakHashMap<>());
36+
}
37+
}
38+
}
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
package org.mozilla.javascript.lc.type.impl.factory;
2+
3+
import org.mozilla.javascript.lc.type.TypeInfo;
4+
import org.mozilla.javascript.lc.type.TypeInfoFactory;
5+
import org.mozilla.javascript.lc.type.impl.BasicClassTypeInfo;
6+
import org.mozilla.javascript.lc.type.impl.EnumTypeInfo;
7+
import org.mozilla.javascript.lc.type.impl.InterfaceTypeInfo;
8+
9+
/**
10+
* {@link ClassValue} is really performant for caching per-class data. However, it's not always
11+
* available on Android. When {@link ClassValue} is not available, {@link LegacyCacheFactory} should
12+
* be used instead.
13+
*
14+
* @see LegacyCacheFactory
15+
* @author ZZZank
16+
*/
17+
public abstract class ClassValueCacheFactory extends AbstractCacheFactory {
18+
private static final ClassValue<TypeInfo> CLASS_TYPE =
19+
new ClassValue<>() {
20+
@Override
21+
protected TypeInfo computeValue(Class<?> type) {
22+
if (type.isEnum()) {
23+
return new EnumTypeInfo(type);
24+
} else if (type.isInterface()) {
25+
return new InterfaceTypeInfo(type);
26+
}
27+
return new BasicClassTypeInfo(type);
28+
}
29+
};
30+
31+
@Override
32+
public TypeInfo create(Class<?> clazz) {
33+
final var predefined = TypeInfoFactory.matchPredefined(clazz);
34+
if (predefined != null) {
35+
return predefined;
36+
} else if (clazz.isArray()) {
37+
return toArray(create(clazz.getComponentType()));
38+
}
39+
return CLASS_TYPE.get(clazz);
40+
}
41+
42+
public static class Concurrent extends ClassValueCacheFactory
43+
implements CachedFactory.Concurrent {}
44+
45+
public static class WeakReference extends ClassValueCacheFactory
46+
implements CachedFactory.WeakReference {}
47+
}

0 commit comments

Comments
 (0)