From 18d2d26882d5cd0fe9cbebe4a66552137c215913 Mon Sep 17 00:00:00 2001 From: Heshan Padmasiri Date: Thu, 3 Oct 2024 07:52:14 +0530 Subject: [PATCH] Refactor type check caching We move the type check cache out of SemType to avoid unnecessary type resolutions --- .../runtime/api/types/semtype/Core.java | 8 +--- .../runtime/api/types/semtype/SemType.java | 31 ------------ .../runtime/internal/TypeChecker.java | 27 +++++++++-- .../runtime/internal/types/BArrayType.java | 2 +- .../runtime/internal/types/BType.java | 48 ++++++++++++++++++- .../types/CacheableTypeDescriptor.java | 19 ++++++++ 6 files changed, 92 insertions(+), 43 deletions(-) create mode 100644 bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/types/CacheableTypeDescriptor.java diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/api/types/semtype/Core.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/api/types/semtype/Core.java index c1018437bb53..7788bb5dae87 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/api/types/semtype/Core.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/api/types/semtype/Core.java @@ -295,13 +295,7 @@ public static boolean isNever(SemType t) { } public static boolean isSubType(Context cx, SemType t1, SemType t2) { - SemType.CachedResult cached = t1.cachedSubTypeRelation(t2); - if (cached != SemType.CachedResult.NOT_FOUND) { - return cached == SemType.CachedResult.TRUE; - } - boolean result = isEmpty(cx, diff(t1, t2)); - t1.cacheSubTypeRelation(t2, result); - return result; + return isEmpty(cx, diff(t1, t2)); } public static boolean isSubtypeSimple(SemType t1, SemType t2) { diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/api/types/semtype/SemType.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/api/types/semtype/SemType.java index 8606126d1d2e..8eea044f57be 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/api/types/semtype/SemType.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/api/types/semtype/SemType.java @@ -5,15 +5,11 @@ import io.ballerina.runtime.internal.types.semtype.MutableSemType; import io.ballerina.runtime.internal.types.semtype.SemTypeHelper; -import java.util.Map; -import java.util.WeakHashMap; - public sealed class SemType extends BasicTypeBitSet permits io.ballerina.runtime.internal.types.BType, ImmutableSemType { private int some; private SubType[] subTypeData; - private volatile Map cachedResults; protected SemType(int all, int some, SubType[] subTypeData) { super(all); @@ -42,33 +38,6 @@ public final SubType[] subTypeData() { return subTypeData; } - public final CachedResult cachedSubTypeRelation(SemType other) { - if (skipCache()) { - return CachedResult.NOT_FOUND; - } - if (cachedResults == null) { - synchronized (this) { - if (cachedResults == null) { - cachedResults = new WeakHashMap<>(); - } - } - return CachedResult.NOT_FOUND; - } - return cachedResults.getOrDefault(other, CachedResult.NOT_FOUND); - } - - private boolean skipCache() { - return this.some() == 0; - } - - public final void cacheSubTypeRelation(SemType other, boolean result) { - if (skipCache() || other.skipCache()) { - return; - } - // we always check of the result before caching so there will always be a map - cachedResults.put(other, result ? CachedResult.TRUE : CachedResult.FALSE); - } - public final SubType subTypeByCode(int code) { if ((some() & (1 << code)) == 0) { return null; diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/TypeChecker.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/TypeChecker.java index 132d6e5fec8c..fcb302493058 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/TypeChecker.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/TypeChecker.java @@ -53,6 +53,7 @@ import io.ballerina.runtime.internal.types.BType; import io.ballerina.runtime.internal.types.BTypeReferenceType; import io.ballerina.runtime.internal.types.BUnionType; +import io.ballerina.runtime.internal.types.CacheableTypeDescriptor; import io.ballerina.runtime.internal.types.TypeWithShape; import io.ballerina.runtime.internal.values.DecimalValue; import io.ballerina.runtime.internal.values.HandleValue; @@ -68,6 +69,7 @@ import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Optional; import java.util.Set; import java.util.function.Predicate; import java.util.stream.Stream; @@ -600,17 +602,36 @@ private static boolean isSubTypeWithInherentType(Context cx, Object sourceValue, } private static boolean isSubType(Type source, Type target) { + if (source instanceof CacheableTypeDescriptor sourceCacheableType && + target instanceof CacheableTypeDescriptor targetCacheableType) { + return isSubTypeWithCache(sourceCacheableType, targetCacheableType); + } // This is really a workaround for Standard libraries that create record types that are not the "same". But // with the same name and expect them to be same. - if (source.equals(target)) { - return true; - } + return isSubTypeInner(source, target); + } + + private static boolean isSubTypeInner(Type source, Type target) { Context cx = context(); SemType sourceSemType = SemType.tryInto(source); SemType targetSemType = SemType.tryInto(target); return Core.isSubType(cx, sourceSemType, targetSemType); } + private static boolean isSubTypeWithCache(CacheableTypeDescriptor source, CacheableTypeDescriptor target) { + if (!source.shouldCache() || !target.shouldCache()) { + return isSubTypeInner(source, target); + } + Optional cachedResult = source.cachedTypeCheckResult(target); + if (cachedResult.isPresent()) { + assert cachedResult.get() == isSubTypeInner(source, target); + return cachedResult.get(); + } + boolean result = isSubTypeInner(source, target); + source.cacheTypeCheckResult(target, result); + return result; + } + private static SemType widenedType(Context cx, Object value) { if (value instanceof BValue bValue) { return bValue.widenedType(); diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/types/BArrayType.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/types/BArrayType.java index 1cf6b859d5fb..8e43ae9fcb14 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/types/BArrayType.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/types/BArrayType.java @@ -148,7 +148,7 @@ public int hashCode() { @Override public boolean equals(Object obj) { if (obj instanceof BArrayType other) { - if (other.state == ArrayState.CLOSED && this.size != other.size) { + if ((other.state == ArrayState.CLOSED || this.state == ArrayState.CLOSED) && this.size != other.size) { return false; } return this.elementType.equals(other.elementType) && this.readonly == other.readonly; diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/types/BType.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/types/BType.java index 01ac5e98905b..58c3b8708973 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/types/BType.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/types/BType.java @@ -28,7 +28,12 @@ import io.ballerina.runtime.internal.TypeChecker; import io.ballerina.runtime.internal.types.semtype.MutableSemType; +import java.util.Map; import java.util.Objects; +import java.util.Optional; +import java.util.WeakHashMap; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; /** * {@code BType} represents a type in Ballerina. @@ -40,7 +45,8 @@ * * @since 0.995.0 */ -public abstract non-sealed class BType extends SemType implements Type, MutableSemType, Cloneable { +public abstract non-sealed class BType extends SemType + implements Type, MutableSemType, Cloneable, CacheableTypeDescriptor { protected String typeName; protected Module pkg; @@ -50,6 +56,8 @@ public abstract non-sealed class BType extends SemType implements Type, MutableS private Type cachedImpliedType = null; private volatile SemType cachedSemType = null; private TypeCreator.TypeMemoKey lookupKey = null; + private volatile Map cachedResults; + private final ReadWriteLock cachedResultsLock = new ReentrantReadWriteLock(); protected BType(String typeName, Module pkg, Class valueClass) { this.typeName = typeName; @@ -285,4 +293,42 @@ public BType clone() { public void setLookupKey(TypeCreator.TypeMemoKey lookupKey) { this.lookupKey = lookupKey; } + + @Override + public boolean shouldCache() { + return true; + } + + @Override + public final Optional cachedTypeCheckResult(CacheableTypeDescriptor other) { + if (other.equals(this)) { + return Optional.of(true); + } + if (cachedResults == null) { + cachedResultsLock.writeLock().lock(); + try { + if (cachedResults == null) { + cachedResults = new WeakHashMap<>(); + } + } finally { + cachedResultsLock.writeLock().unlock(); + } + } + cachedResultsLock.readLock().lock(); + try { + return Optional.ofNullable(cachedResults.get(other)); + } finally { + cachedResultsLock.readLock().unlock(); + } + } + + @Override + public final void cacheTypeCheckResult(CacheableTypeDescriptor other, boolean result) { + cachedResultsLock.writeLock().lock(); + try { + cachedResults.put(other, result); + } finally { + cachedResultsLock.writeLock().unlock(); + } + } } diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/types/CacheableTypeDescriptor.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/types/CacheableTypeDescriptor.java new file mode 100644 index 000000000000..3897db2a1f03 --- /dev/null +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/types/CacheableTypeDescriptor.java @@ -0,0 +1,19 @@ +package io.ballerina.runtime.internal.types; + +import io.ballerina.runtime.api.types.Type; + +import java.util.Optional; + +/** + * Represent TypeDescriptors whose type check results can be cached + * + * @since 2201.11.0 + */ +public interface CacheableTypeDescriptor extends Type { + + boolean shouldCache(); + + Optional cachedTypeCheckResult(CacheableTypeDescriptor other); + + void cacheTypeCheckResult(CacheableTypeDescriptor other, boolean result); +}