Skip to content

Commit d32b2ef

Browse files
Christopher-ChianelliChristopher-Chianelli
committed
chore: use Gizmo MemberAccessors when possible, remove domainAccessType from SolverConfig
Note: SolutionCloner will still use REFLECTION by default, since GIZMO cannot copy final fields and requires all fields to be accessible (i.e. either public or have public getters and setters).
1 parent 69ccead commit d32b2ef

53 files changed

Lines changed: 468 additions & 571 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎core/pom.xml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
<dependency>
3333
<groupId>io.quarkus.gizmo</groupId>
3434
<artifactId>gizmo2</artifactId>
35-
<!-- <optional>true</optional>-->
3635
</dependency>
3736

3837
<!-- External dependencies -->

‎core/src/main/java/ai/timefold/solver/core/config/solver/SolverConfig.java‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import jakarta.xml.bind.annotation.XmlTransient;
2727
import jakarta.xml.bind.annotation.XmlType;
2828

29-
import ai.timefold.solver.core.api.domain.common.DomainAccessType;
3029
import ai.timefold.solver.core.api.domain.solution.cloner.SolutionCloner;
3130
import ai.timefold.solver.core.api.score.calculator.EasyScoreCalculator;
3231
import ai.timefold.solver.core.api.score.stream.ConstraintProvider;
@@ -73,7 +72,6 @@
7372
"monitoringConfig",
7473
"solutionClass",
7574
"entityClassList",
76-
"domainAccessType",
7775
"scoreDirectorFactoryConfig",
7876
"terminationConfig",
7977
"nearbyDistanceMeterClass",
@@ -228,7 +226,6 @@ public class SolverConfig extends AbstractConfig<SolverConfig> {
228226

229227
@XmlElement(name = "entityClass")
230228
protected List<Class<?>> entityClassList = null;
231-
protected DomainAccessType domainAccessType = null;
232229
@XmlTransient
233230
protected Map<String, MemberAccessor> gizmoMemberAccessorMap = null;
234231
@XmlTransient
@@ -399,14 +396,6 @@ public void setEntityClassList(@Nullable List<Class<?>> entityClassList) {
399396
this.entityClassList = entityClassList;
400397
}
401398

402-
public @Nullable DomainAccessType getDomainAccessType() {
403-
return domainAccessType;
404-
}
405-
406-
public void setDomainAccessType(@Nullable DomainAccessType domainAccessType) {
407-
this.domainAccessType = domainAccessType;
408-
}
409-
410399
public @Nullable Map<@NonNull String, @NonNull MemberAccessor> getGizmoMemberAccessorMap() {
411400
return gizmoMemberAccessorMap;
412401
}
@@ -527,11 +516,6 @@ public void setMonitoringConfig(@Nullable MonitoringConfig monitoringConfig) {
527516
return this;
528517
}
529518

530-
public @NonNull SolverConfig withDomainAccessType(@NonNull DomainAccessType domainAccessType) {
531-
this.domainAccessType = domainAccessType;
532-
return this;
533-
}
534-
535519
public @NonNull SolverConfig
536520
withGizmoMemberAccessorMap(@NonNull Map<@NonNull String, @NonNull MemberAccessor> memberAccessorMap) {
537521
this.gizmoMemberAccessorMap = memberAccessorMap;
@@ -651,10 +635,6 @@ public boolean canTerminate() {
651635
return Objects.requireNonNullElse(environmentMode, EnvironmentMode.PHASE_ASSERT);
652636
}
653637

654-
public @NonNull DomainAccessType determineDomainAccessType() {
655-
return Objects.requireNonNullElse(domainAccessType, DomainAccessType.REFLECTION);
656-
}
657-
658638
public @NonNull MonitoringConfig determineMetricConfig() {
659639
return Objects.requireNonNullElse(monitoringConfig,
660640
new MonitoringConfig().withSolverMetricList(Arrays.asList(SolverMetric.SOLVE_DURATION, SolverMetric.ERROR_COUNT,
@@ -698,7 +678,6 @@ public void offerRandomSeedFromSubSingleIndex(long subSingleIndex) {
698678
solutionClass = ConfigUtils.inheritOverwritableProperty(solutionClass, inheritedConfig.getSolutionClass());
699679
entityClassList = ConfigUtils.inheritMergeableListProperty(entityClassList,
700680
inheritedConfig.getEntityClassList());
701-
domainAccessType = ConfigUtils.inheritOverwritableProperty(domainAccessType, inheritedConfig.getDomainAccessType());
702681
gizmoMemberAccessorMap = ConfigUtils.inheritMergeableMapProperty(
703682
gizmoMemberAccessorMap, inheritedConfig.getGizmoMemberAccessorMap());
704683
gizmoSolutionClonerMap = ConfigUtils.inheritMergeableMapProperty(

‎core/src/main/java/ai/timefold/solver/core/config/util/ConfigUtils.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,10 @@
3131
import java.util.stream.Collectors;
3232
import java.util.stream.Stream;
3333

34-
import ai.timefold.solver.core.api.domain.common.DomainAccessType;
3534
import ai.timefold.solver.core.api.domain.lookup.PlanningId;
3635
import ai.timefold.solver.core.config.AbstractConfig;
3736
import ai.timefold.solver.core.impl.domain.common.AlphabeticMemberComparator;
37+
import ai.timefold.solver.core.impl.domain.common.DomainAccessType;
3838
import ai.timefold.solver.core.impl.domain.common.ReflectionHelper;
3939
import ai.timefold.solver.core.impl.domain.common.accessor.MemberAccessor;
4040
import ai.timefold.solver.core.impl.domain.common.accessor.MemberAccessorFactory;

core/src/main/java/ai/timefold/solver/core/api/domain/common/DomainAccessType.java renamed to core/src/main/java/ai/timefold/solver/core/impl/domain/common/DomainAccessType.java

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package ai.timefold.solver.core.api.domain.common;
1+
package ai.timefold.solver.core.impl.domain.common;
22

33
import ai.timefold.solver.core.api.domain.variable.PlanningVariable;
44

@@ -7,25 +7,26 @@
77
* are accessed.
88
*/
99
public enum DomainAccessType {
10+
/**
11+
* Determine what domain access type to use automatically.
12+
* <p>
13+
* This is the default.
14+
*/
15+
AUTO,
16+
1017
/**
1118
* Use reflection to read/write members (fields and methods) of the domain.
1219
* <p>
1320
* When used in a modulepath, the module must be open.
1421
* When used in GraalVM, the domain must be open for reflection.
15-
* <p>
16-
* This is the default, except with timefold-solver-quarkus.
1722
*/
1823
REFLECTION,
24+
1925
/**
2026
* Use Gizmo generated bytecode to access members (fields and methods) to avoid reflection
2127
* for additional performance.
2228
* <p>
23-
* With timefold-solver-quarkus, this bytecode is generated at build time
24-
* and it supports planning annotations on non-public members too.
25-
* <p>
26-
* Without timefold-solver-quarkus, this bytecode is generated at bootstrap runtime
27-
* and you must add Gizmo in your classpath or modulepath
28-
* and use planning annotations on public members only.
29+
* This is the default when the application is run inside a JVM and not a native image.
2930
*/
3031
GIZMO
3132
}

‎core/src/main/java/ai/timefold/solver/core/impl/domain/common/accessor/MemberAccessorFactory.java‎

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,22 @@
1010
import java.util.Objects;
1111
import java.util.concurrent.ConcurrentHashMap;
1212

13-
import ai.timefold.solver.core.api.domain.common.DomainAccessType;
1413
import ai.timefold.solver.core.api.solver.SolverFactory;
14+
import ai.timefold.solver.core.impl.domain.common.DomainAccessType;
1515
import ai.timefold.solver.core.impl.domain.common.ReflectionHelper;
1616
import ai.timefold.solver.core.impl.domain.common.accessor.gizmo.AccessorInfo;
1717
import ai.timefold.solver.core.impl.domain.common.accessor.gizmo.GizmoClassLoader;
1818
import ai.timefold.solver.core.impl.domain.common.accessor.gizmo.GizmoMemberAccessorFactory;
1919

20+
import org.jspecify.annotations.NullMarked;
21+
import org.jspecify.annotations.Nullable;
22+
import org.slf4j.Logger;
23+
import org.slf4j.LoggerFactory;
24+
25+
@NullMarked
2026
public final class MemberAccessorFactory {
2127

28+
static final Logger LOGGER = LoggerFactory.getLogger(MemberAccessorFactory.class);
2229
// exists only so that the various member accessors can share the same text in their exception messages
2330
static final String CLASSLOADER_NUDGE_MESSAGE =
2431
"Maybe add getClass().getClassLoader() as a parameter to the %s.create...() method call."
@@ -33,7 +40,7 @@ public final class MemberAccessorFactory {
3340
* @param classLoader null or {@link GizmoClassLoader} if domainAccessType is {@link DomainAccessType#GIZMO}.
3441
* @return never null, new instance of the member accessor
3542
*/
36-
public static MemberAccessor buildMemberAccessor(Member member, MemberAccessorType memberAccessorType,
43+
private static MemberAccessor buildMemberAccessor(Member member, MemberAccessorType memberAccessorType,
3744
DomainAccessType domainAccessType, ClassLoader classLoader) {
3845
return buildMemberAccessor(member, memberAccessorType, null, domainAccessType, classLoader);
3946
}
@@ -48,25 +55,27 @@ public static MemberAccessor buildMemberAccessor(Member member, MemberAccessorTy
4855
* @param classLoader null or {@link GizmoClassLoader} if domainAccessType is {@link DomainAccessType#GIZMO}.
4956
* @return never null, new instance of the member accessor
5057
*/
51-
public static MemberAccessor buildMemberAccessor(Member member, MemberAccessorType memberAccessorType,
52-
Class<? extends Annotation> annotationClass, DomainAccessType domainAccessType, ClassLoader classLoader) {
58+
static MemberAccessor buildMemberAccessor(Member member, MemberAccessorType memberAccessorType,
59+
@Nullable Class<? extends Annotation> annotationClass, DomainAccessType domainAccessType, ClassLoader classLoader) {
5360
return switch (domainAccessType) {
61+
case AUTO -> throw new IllegalStateException(
62+
"Impossible state: called with %s (AUTO) instead of a resolved domain access type"
63+
.formatted(DomainAccessType.class.getSimpleName()));
5464
case GIZMO -> GizmoMemberAccessorFactory.buildGizmoMemberAccessor(member, annotationClass,
55-
AccessorInfo.of(memberAccessorType != MemberAccessorType.VOID_METHOD,
56-
memberAccessorType == MemberAccessorType.FIELD_OR_READ_METHOD_WITH_OPTIONAL_PARAMETER),
65+
AccessorInfo.of(memberAccessorType),
5766
(GizmoClassLoader) Objects.requireNonNull(classLoader));
5867
case REFLECTION -> buildReflectiveMemberAccessor(member, memberAccessorType, annotationClass);
5968
};
6069
}
6170

6271
private static MemberAccessor buildReflectiveMemberAccessor(Member member, MemberAccessorType memberAccessorType,
63-
Class<? extends Annotation> annotationClass) {
72+
@Nullable Class<? extends Annotation> annotationClass) {
6473
return buildReflectiveMemberAccessor(member, memberAccessorType, annotationClass,
6574
(AnnotatedElement) member);
6675
}
6776

6877
private static MemberAccessor buildReflectiveMemberAccessor(Member member, MemberAccessorType memberAccessorType,
69-
Class<? extends Annotation> annotationClass, AnnotatedElement annotatedElement) {
78+
@Nullable Class<? extends Annotation> annotationClass, AnnotatedElement annotatedElement) {
7079
var messagePrefix = (annotationClass == null) ? "The" : "The @%s annotated".formatted(annotationClass.getSimpleName());
7180
if (member instanceof Field field) {
7281
var getter = ReflectionHelper.getGetterMethod(field.getDeclaringClass(), field.getName());
@@ -154,6 +163,7 @@ private static MemberAccessor buildReflectiveMemberAccessor(Member member, Membe
154163

155164
private final Map<String, MemberAccessor> memberAccessorCache;
156165
private final GizmoClassLoader gizmoClassLoader = new GizmoClassLoader();
166+
private final boolean isGizmoSupported;
157167

158168
public MemberAccessorFactory() {
159169
this(null);
@@ -164,10 +174,16 @@ public MemberAccessorFactory() {
164174
*
165175
* @param memberAccessorMap key is the fully qualified member name
166176
*/
167-
public MemberAccessorFactory(Map<String, MemberAccessor> memberAccessorMap) {
177+
public MemberAccessorFactory(@Nullable Map<String, MemberAccessor> memberAccessorMap) {
168178
// The MemberAccessorFactory may be accessed, and this cache both read and updated, by multiple threads.
169179
this.memberAccessorCache =
170180
memberAccessorMap == null ? new ConcurrentHashMap<>() : new ConcurrentHashMap<>(memberAccessorMap);
181+
// If the memberAccessorMap is not empty, we are in Quarkus using pregenerated member accessors
182+
this.isGizmoSupported =
183+
(memberAccessorMap != null && !memberAccessorMap.isEmpty()) || GizmoMemberAccessorFactory.isGizmoSupported(
184+
gizmoClassLoader);
185+
LOGGER.trace("Using domain access type {} for member accessors",
186+
isGizmoSupported ? DomainAccessType.GIZMO : DomainAccessType.REFLECTION);
171187
}
172188

173189
/**
@@ -180,10 +196,16 @@ public MemberAccessorFactory(Map<String, MemberAccessor> memberAccessorMap) {
180196
* @return never null, new {@link MemberAccessor} instance unless already found in memberAccessorMap
181197
*/
182198
public MemberAccessor buildAndCacheMemberAccessor(Member member, MemberAccessorType memberAccessorType,
183-
Class<? extends Annotation> annotationClass, DomainAccessType domainAccessType) {
199+
@Nullable Class<? extends Annotation> annotationClass, DomainAccessType domainAccessType) {
184200
String generatedClassName = GizmoMemberAccessorFactory.getGeneratedClassName(member);
201+
if (domainAccessType == DomainAccessType.AUTO) {
202+
domainAccessType = isGizmoSupported ? DomainAccessType.GIZMO : DomainAccessType.REFLECTION;
203+
}
204+
205+
var finalDomainAccessType = domainAccessType;
185206
return memberAccessorCache.computeIfAbsent(generatedClassName,
186-
k -> MemberAccessorFactory.buildMemberAccessor(member, memberAccessorType, annotationClass, domainAccessType,
207+
k -> MemberAccessorFactory.buildMemberAccessor(member, memberAccessorType, annotationClass,
208+
finalDomainAccessType,
187209
gizmoClassLoader));
188210
}
189211

@@ -198,8 +220,14 @@ public MemberAccessor buildAndCacheMemberAccessor(Member member, MemberAccessorT
198220
public MemberAccessor buildAndCacheMemberAccessor(Member member, MemberAccessorType memberAccessorType,
199221
DomainAccessType domainAccessType) {
200222
String generatedClassName = GizmoMemberAccessorFactory.getGeneratedClassName(member);
223+
if (domainAccessType == DomainAccessType.AUTO) {
224+
domainAccessType = isGizmoSupported ? DomainAccessType.GIZMO : DomainAccessType.REFLECTION;
225+
}
226+
227+
var finalDomainAccessType = domainAccessType;
201228
return memberAccessorCache.computeIfAbsent(generatedClassName,
202-
k -> MemberAccessorFactory.buildMemberAccessor(member, memberAccessorType, domainAccessType, gizmoClassLoader));
229+
k -> MemberAccessorFactory.buildMemberAccessor(member, memberAccessorType, finalDomainAccessType,
230+
gizmoClassLoader));
203231
}
204232

205233
public GizmoClassLoader getGizmoClassLoader() {

‎core/src/main/java/ai/timefold/solver/core/impl/domain/common/accessor/ReflectionBeanPropertyMemberAccessor.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public ReflectionBeanPropertyMemberAccessor(Method getterMethod, AnnotatedElemen
3434
this.annotatedElement = annotatedElement;
3535
MethodHandles.Lookup lookup = MethodHandles.lookup();
3636
try {
37-
getterMethod.setAccessible(true);
37+
this.getterMethod.setAccessible(true);
3838
this.getherMethodHandle = lookup.unreflect(getterMethod)
3939
.asFixedArity();
4040
} catch (IllegalAccessException e) {
@@ -73,7 +73,7 @@ public ReflectionBeanPropertyMemberAccessor(Method getterMethod, AnnotatedElemen
7373
declaringClass.getCanonicalName()));
7474
}
7575
try {
76-
setterMethod.setAccessible(true);
76+
this.setterMethod.setAccessible(true);
7777
this.setterMethodHandle = lookup.unreflect(setterMethod)
7878
.asFixedArity();
7979
} catch (IllegalAccessException e) {

‎core/src/main/java/ai/timefold/solver/core/impl/domain/common/accessor/gizmo/AccessorInfo.java‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,27 @@
11
package ai.timefold.solver.core.impl.domain.common.accessor.gizmo;
22

3+
import ai.timefold.solver.core.impl.domain.common.accessor.MemberAccessorFactory;
4+
35
/**
46
* Additional information for the GIZMO accessor generation.
57
*
68
* @param returnTypeRequired a flag that indicates if the return type is required or optional
79
* @param readMethodWithParameter a flag that allows the read method to accept an argument
810
*/
9-
public record AccessorInfo(boolean returnTypeRequired, boolean readMethodWithParameter) {
11+
public record AccessorInfo(MemberAccessorFactory.MemberAccessorType memberAccessorType, boolean returnTypeRequired,
12+
boolean readMethodWithParameter) {
1013

1114
public static AccessorInfo withReturnValueAndNoArguments() {
12-
return new AccessorInfo(true, false);
15+
return new AccessorInfo(MemberAccessorFactory.MemberAccessorType.FIELD_OR_READ_METHOD, true, false);
1316
}
1417

1518
public static AccessorInfo withReturnValueAndArguments() {
16-
return new AccessorInfo(true, true);
19+
return new AccessorInfo(MemberAccessorFactory.MemberAccessorType.FIELD_OR_READ_METHOD_WITH_OPTIONAL_PARAMETER, true,
20+
true);
1721
}
1822

19-
public static AccessorInfo of(boolean returnTypeRequired, boolean readMethodWithParameter) {
20-
return new AccessorInfo(returnTypeRequired, readMethodWithParameter);
23+
public static AccessorInfo of(MemberAccessorFactory.MemberAccessorType memberAccessorType) {
24+
return new AccessorInfo(memberAccessorType, memberAccessorType != MemberAccessorFactory.MemberAccessorType.VOID_METHOD,
25+
memberAccessorType == MemberAccessorFactory.MemberAccessorType.FIELD_OR_READ_METHOD_WITH_OPTIONAL_PARAMETER);
2126
}
2227
}

‎core/src/main/java/ai/timefold/solver/core/impl/domain/common/accessor/gizmo/GizmoClassLoader.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
public final class GizmoClassLoader extends ClassLoader {
1212

1313
private final Map<String, byte[]> classNameToBytecodeMap;
14+
private GizmoSupportStatus gizmoSupportStatus;
1415

1516
public GizmoClassLoader() {
1617
this(new HashMap<>());
@@ -24,6 +25,15 @@ public GizmoClassLoader(Map<String, byte[]> classNameToBytecodeMap) {
2425
*/
2526
super(GizmoClassLoader.class.getClassLoader());
2627
this.classNameToBytecodeMap = classNameToBytecodeMap;
28+
this.gizmoSupportStatus = GizmoSupportStatus.UNKNOWN;
29+
}
30+
31+
public GizmoSupportStatus getGizmoSupportStatus() {
32+
return gizmoSupportStatus;
33+
}
34+
35+
public void setGizmoSupportStatus(GizmoSupportStatus gizmoSupportStatus) {
36+
this.gizmoSupportStatus = gizmoSupportStatus;
2737
}
2838

2939
@Override

‎core/src/main/java/ai/timefold/solver/core/impl/domain/common/accessor/gizmo/GizmoFieldHandler.java‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,18 +36,20 @@ final class GizmoFieldHandler implements GizmoMemberHandler {
3636
.formatted(fieldDescriptor.name(), declaringClass.getName()));
3737
}
3838
if (!ignoreChecks && !isFieldPublic) {
39-
throw new IllegalStateException("""
40-
Member (%s) of class (%s) is not public."""
41-
.formatted(fieldDescriptor.name(), declaringClass.getName()));
39+
throw new IllegalArgumentException(
40+
"""
41+
Member (%s) of class (%s) is not public."""
42+
.formatted(fieldDescriptor.name(), declaringClass.getName()));
4243
}
4344
getterDescriptor = null;
4445
setterDescriptor = null;
4546
this.canBeWritten = canBeWritten;
4647
} else {
4748
if (!ignoreChecks && !Modifier.isPublic(getterMethod.getModifiers())) {
48-
throw new IllegalStateException("""
49-
Member (%s) of class (%s) is not public."""
50-
.formatted(getterMethod.getName(), getterMethod.getDeclaringClass().getName()));
49+
throw new IllegalArgumentException(
50+
"""
51+
Member (%s) of class (%s) is not public."""
52+
.formatted(getterMethod.getName(), getterMethod.getDeclaringClass().getName()));
5153
}
5254
ReflectionHelper.assertGetterMethod(getterMethod);
5355
getterDescriptor = MethodDesc.of(getterMethod);

0 commit comments

Comments
 (0)