Improve Kotlin support

[greyhairredbear]

Im currently working on my second project (first one in a scheduling domain, second one a VRP) using kotlin as programming language and optaplanner as optimization library.

Generally, being able to use Kotlin feels like a big productivity boost when writing domain logic or application logic. However, when it comes to integrating optaplanner, a few things don't really feel idomatic:

• Mutability: A lot of members in classes have to be vars
• Required no-args constructurs lead to lots of lateinit vars (or alternatively nullable vals, which theoretically could be vals
• Nullability required sometimes
• ... (open to suggestions :) )

I completely understand that most of these issues arise due to optaplanner/timefold being written in Java and performance being a main concern. It still would be pretty nice if there was the possibility to use timefold in a more kotlin-idiomatic way than it was/is possible with optaplanner.

That leads me to some questions:

• Is there any data how many people use optaplanner together with kotlin? Would it be worthwile to spend effort here?
• Have there been thoughts or discussions regarding improving support for kotlin?
• What would be the best way to proceed if one was to trying to improve kotlin support with the mentioned topics? First, I was thinking about creating a kotlin DSL wrapper for optaplanner, but I might be missing some relevant points here. There also might be the possibility of writing a compiler plugin, but I my experience with that is pretty much zero.

I'd be interested to contribute to this topic, maybe also start with an external project as POC. Glad about any hints and ideas :)

[triceo]

Hey there,

interesting observations. We do not know how many of our users are Kotlin-based. We occasionally get a Kotlin-related question, and we get much less of those than Java questions, but that is the only data point we have.

Wrt. some of your other questions:

• Mutability is indeed due to performance; if we had to clone entities every time we changed planning variables, that would destroy our performance. This operation happens millions of times per second; it needs to be fast and not produce garbage. Immutability would be the wrong choice here, in my opinion.

• Nullability doesn't have to do with Java, or with performance; rather, it is an easy way how to express that a planning variable can be unassigned. Had we designed this API in 2020 and not in 2010, we'd maybe have chosen Optional for it. At this point, forcing users to make the switch - in my opinion - brings little benefit.

• Wrt. default no-arg constructors - I am not too excited about those either. However, since everything we do needs to be fully serializable to XML, JSON etc., we once again have no choice. (Other than perhaps creating a lot of boilerplate per entity.)

Now, to Kotlin support in particular. I am open to considering any change which improves life for both Java users and Kotlin users. However, maintaining any Kotlin-specific bindings or duplicating parts of the codebase with Kotlin-specific code paths is not what I would chose. I would have to be strongly convinced that this brings major benefits, especially if it meant introducing Kotlin to our codebase.

As Java continues to evolve, and especially with project Valhalla, I expect we will have to reconsider several of the decisions discussed here. (Mainly nullability and mutability.) When the time comes, we may come with major new APIs, which are going to address the problems for both Java users and Kotlin users. That would be my preferred way of addressing these issues. (And honestly, I can't wait.)

In summation: I am open to any pull requests which improve the lives of our users, but those benefits will be weighed on a case-by-case basis against the costs of long-term maintenance. Let's talk about the individual changes you want to propose, and let's discuss them on their merits.

(Finally, my personal opinion: I do not consider being able to write nicer code to be of high enough value to offset runtime performance and maintenance considerations. "Nice" is a matter of personal preference, and "idiomatic" is a fancy word for saying "we want it like this because we think it's nice like this".)

[triceo]

I'd be interested to contribute to this topic, maybe also start with an external project as POC. Glad about any hints and ideas :)

PoCs are always welcome! A Kotlin DSL would probably be the way to go - even if only as a way to see where the pain points are, to know what to address in Timefold directly.

[ge0ffrey]

Thank you for reporting.

1. We can fix nullability with Nullable and NotNull annotations. I have a POC of that.

2. Maybe we can fix nor arg constructors and non-plannning-variable mutability with a variant of @JsonCreate on the constructor so the pjanning cloner can still do its work on planning entities.

[greyhairredbear]

Thank you both for the quick replies and the useful information!

In summation: I am open to any pull requests which improve the lives of our users, but those benefits will be weighed on a case-by-case basis against the costs of long-term maintenance. Let's talk about the individual changes you want to propose, and let's discuss them on their merits.

That sounds like a good plan :)

However, maintaining any Kotlin-specific bindings or duplicating parts of the codebase with Kotlin-specific code paths is not what I would chose. I would have to be strongly convinced that this brings major benefits, especially if it meant introducing Kotlin to our codebase.

I can completely understand that and I also wouldn't want that in my own codebase. What I meant with "DSL wrapper" before could maybe be achieved in a different project and wouldn't have to be a part of timefold-solver. But I suppose I would have to try to find out.

(Finally, my personal opinion: I do not consider being able to write nicer code to be of high enough value to offset runtime performance and maintenance considerations. "Nice" is a matter of personal preference, and "idiomatic" is a fancy word for saying "we want it like this because we think it's nice like this".)

I agree to some degree, but I also think "nice" (which I would interpret as readable and robust) code plays an important role not only when expanding a codebase, but especially with maintenance (of programs using timefold) in mind. And that's possible in many programming languages. Regarding "idiomatic" code: My opinion here is that adhering to frequent patterns and conventions found in a programming language makes it easier to integrate new people into a team. So in the end, it's about standards. Still, I can completely understand that from the standpoint of an optimization library developer runtime performance and (library) maintenance outweigh ergonomics of use (especially if the number of Kotlin users is comparatively small).

We can fix nullability with Nullable and NotNull annotations. I have a POC of that.

If there is any assistance wanted here, I'd be glad to open a PR. Also seems like a rather low hanging fruit to me.

Maybe we can fix nor arg constructors and non-plannning-variable mutability with a variant of @JsonCreate on the constructor so the planning cloner can still do its work on planning entities.

I think I'd really like getting rid of no-args-ctors. However, as of now, I don't know nearly enough about internal details of timefold to be helpful here. I still would be willing to try, but no guarantees :)

I think, these two changes (Nullable/NotNull annotations and removing no-arg ctors) would already be very convenient to have and the second one would also partially address immutability (in the sense that lateinit vars would not be required anymore).

[triceo]

We can fix nullability with Nullable and NotNull annotations. I have a POC of that.

If there is any assistance wanted here, I'd be glad to open a PR. Also seems like a rather low hanging fruit to me.

You can try providing a PR adding the annotations in places where you see fit. We can then review the full extent of those changes and discuss if that's something we're willing to maintain upstream in perpetuity.

I think I'd really like getting rid of no-args-ctors. However, as of now, I don't know nearly enough about internal details of timefold to be helpful here. I still would be willing to try, but no guarantees :)

I've been thinking about that a bit more and I really don't think this applies to the solver; rather, it applies to the examples and quickstarts, which is user code. In other ways - you should be able to do without no-arg-ctors if you choose to write code like that. However, maybe I forgot/did not expect some places. If you list places where we force you to use no-arg constructors, we can discuss what to do about those.

[Inlustra]

We've just recently been migrating toward Kotlin to get the syntax a little closer to the rest of our stack which is written in Typescript.
It's been an interesting challenge trying to work out what can be nullable business logic vs what must be nullable due to the current requirement behind no-arg constructors etc.

We're still mid-migration, but just wanted to add that we are also using Kotlin now and would love to see the PoC @ge0ffrey

[greyhairredbear]

I've been thinking about that a bit more and I really don't think this applies to the solver; rather, it applies to the examples and quickstarts, which is user code. In other ways - you should be able to do without no-arg-ctors if you choose to write code like that.

It seems I probably misunderstood @ge0ffrey before. If I just use @JsonCreate (is that from Jackson btw?) to allow proper deserialization, the the no-arg ctor shouldn't be required? That could actually be quite "nice" ;)

It's been an interesting challenge trying to work out what can be nullable business logic vs what must be nullable due to the current requirement behind no-arg constructors etc.

I suppose the PoC with nullability annotations would mainly improve the API that interacts with Timefols directly. In the Javadoc, there are already lots of parameters denoted as nullable/non-null - it's just that the annotations required by kotlin to corrctly interpret a type as nullable or non-nullable have never been added

[triceo]

It seems I probably misunderstood @ge0ffrey before. If I just use @JsonCreate (is that from Jackson btw?) to allow proper deserialization, the the no-arg ctor shouldn't be required? That could actually be quite "nice" ;)

I think that it would remove part of the pain, yes. But I'm not sure how our cloners would deal with such entities; that remains to be tried. (You can write your own cloners, so that part of the pain can be eliminated too, but writing cloners is a messy business, and definitely boilerplate. Much worse than having to deal with no-arg constructors, in my opinion.)

[ge0ffrey]

For nullable annotations, this approach worked well (very similar to how Spring became Kotlin friendly):
https://github.com/kiegroup/optaplanner/pull/760/files
The only reason it wasn't finished is because of the extra dependency that we need to pull in: at the time, JetBrains didn't have a canonical answer on that.

[ge0ffrey]

"We need to get them from JSpecify (credit: Anton at JFokus)"
That solves that, I guess.

[ge0ffrey]

Kotlin notebook thread: #384

[greyhairredbear]

Note on the constructor/immutability part of this discussion:
I've tried to get rid of the no-args constructors by using the suggested annotations for my planning classes, but unfortunately, I didn't succeed there.

However, I remembered the no-arg gradle plugin for kotlin which for whatever reason I didn't have on my mind when starting this discussion back in August.

Disclaimer (should be noticeable from the imports though): The project I'm working on is still using optaplanner - However, I think this should also apply to Timefold.

Using the following configuration in my build.gradle

noArg {
    annotation("org.optaplanner.core.api.domain.solution.cloner.DeepPlanningClone")
    annotation("org.optaplanner.core.api.domain.entity.PlanningEntity")
    annotation("org.optaplanner.core.api.domain.solution.PlanningSolution")
}

I was able to rewrite something like this PlanningEntity for a VRP

@PlanningEntity
class Tour {
    @PlanningId lateinit var planningId: String

    @PlanningListVariable lateinit var actions: MutableList<Action>

    lateinit var vehicle: Vehicle

    lateinit var startLocation: Location
        private set

    var endLocation: Location? = null
        private set

    constructor() // required for optaplanner

    constructor(
        planningId: String,
        actions: List<Action>,
        vehicle: Vehicle,
        startLocation: Location,
        endLocation: Location?,
    ) {
        this.planningId = planningId
        this.actions = actions.toMutableList()
        this.vehicle = vehicle
        this.startLocation = startLocation
        this.endLocation = endLocation
    }
}

to this:

@PlanningEntity
class Tour(
    @PlanningId val planningId: String,
    actions: List<Action>,
    val vehicle: Vehicle,
    val startLocation: Location,
    val endLocation: Location?,
) {
    @PlanningListVariable val actions: MutableList<Action> = actions.toMutableList()
}

Which basically checks

• Mutability: A lot of members in classes have to be vars
• Required no-args constructurs lead to lots of lateinit vars (or alternatively nullable vals, which theoretically could be vals

It might be worth mentioning this in the documentation or adjusting the kotlin quickstarts. Maybe (don't know if it'd be overkill or if it would be even possible to configure a compiler plugin this way) even something like a separate timefold-kotlin dependency which would configure no-args constructors for @PlanningEntity etc. would be nice.

I'd be glad to help on this when I have some spare time left between work and family - provided adjustments like these would be endorsed by the timefold team. Any thoughts @ge0ffrey @triceo ?

[triceo]

I struggle with this; this is magical. Somewhere some foreign code performs something to your classes, it's not obvious who did it, how it got done and what exactly got done.

What are the performance characteristics of the current code vs. the new code?
What do I see when I look at the code via the debugger? How do I put a breakpoint in a constructor which does not exist?
What do I see in the stack trace?
Is this level of magic and obfuscation at runtime really worth a bit of sugar in the source code?

I am not convinced yet. I can be convinced, but first I'd like to fully understand the implications.

[greyhairredbear]

You've got some very good points here and I understand your concerns. I'll try to answer to the best of my knowledge and give some further information. Also, I think I might need to do some research, do some measurements and look at the bytecode to provide better answers.

I struggle with this; this is magical. Somewhere some foreign code performs something to your classes, it's not obvious who did it, how it got done and what exactly got done.

The docs for the no-arg plugin I linked in the post above (https://kotlinlang.org/docs/no-arg-plugin.html) state

The no-arg compiler plugin generates an additional zero-argument constructor for classes with a specific annotation.

The generated constructor is synthetic, so it can't be directly called from Java or Kotlin, but it can be called using reflection.

This makes it rather transparent to me what this plugin does: Look for classes with the configured annotations and generate a synthetic no-arg constructor for them. Also, you have to explicitly add this plugin to your build file (be it gradle or maven) and configure it as described above. Another point here: When working with Kotlin and JPA, using the noarg (and the allopen) plugin would be considered pretty much standard. So there is some familiarity with this in the community and since these are official Kotlin plugins, I'd even consider them part of the language (or at least the kotlin-jvm flavor of it).

That all being said, I'm also rather unhappy that this plugin is required to get rid of boilerplate when using JPA or optaplanner - but provided there are no repercussions when using this plugin (and I never ran into issues when using it together with spring and jpa), it's definitely the lesser evil to me.

What are the performance characteristics of the current code vs. the new code?

I'm not 100% sure here, so I'll definitely run the profiler and the benchmarker on that.

• Regarding the access of the no-arg constructor: From what I understand and read in the (Optaplanner) source code (classes FieldAccessingSolutionCloner and FieldCloningUtils), reflection is used to access and invoke the constructor and then copy the field values over to the new instance. I suppose it shouldn't make any difference if one is retrieving a synthetic or a regular constructor using reflection.
  • note: I know gizmo is used as a member accessor in some parts of the code, I don't know if this also applies to solution cloning...
• Regarding performance of a synthetic constructor: My knowledge of the jvm isn't deep enough to make any claims here, but again, I wouldn't expect a synthetic constructor to be slower.

What do I see when I look at the code via the debugger? How do I put a breakpoint in a constructor which does not exist?

I don't know, but I'm more than happy to try that out. I'd assume you can't. I don't know if IntelliJ allows putting breakpoints into bytecode.

What do I see in the stack trace?

Again, don't know, but happy to investigate. I think I'll take some time and try to get a constructor generated by noarg to crash and see what that gets me.

Is this level of magic and obfuscation at runtime really worth a bit of sugar in the source code?

I think I've at least partially answered that in the first section. Also, I feel this part of the discussion is related to the one we had a while ago in the comments above regarding "idiomatic" code. Taking this to extremes, one could even argue that Kotlin on the JVM is nothing more than syntactic sugar. I understand that it's never an easy task to weigh costs and benefits in a scenario like this and I certainly don't know the Timefold/Optaplanner community as well as you guys do.

I can just speak for myself when I say that (assuming there are no or negligible performance impacts with the noarg-plugin approach) I would have liked to know about having an alternative when starting off.

[triceo]

When working with Kotlin and JPA, using the noarg (and the allopen) plugin would be considered pretty much standard. So there is some familiarity with this in the community and since these are official Kotlin plugins, I'd even consider them part of the language (or at least the kotlin-jvm flavor of it).

Good point. This is already a very strong argument for including this in the quickstart; if this is what the ecosystem typically does, who am I to object.

I suppose it shouldn't make any difference if one is retrieving a synthetic or a regular constructor using reflection.

I agree.

I don't know, but I'm more than happy to try that out. I'd assume you can't. I don't know if IntelliJ allows putting breakpoints into bytecode.

It would be interesting to understand what happens when IntelliJ decompiles the class.
Not being able to put a breakpoint in the constructor is unfortunate, but not a dealbreaker I'd argue.

What do I see in the stack trace?

Again, don't know, but happy to investigate. I think I'll take some time and try to get a constructor generated by noarg to crash and see what that gets me.

Thank you.

Taking this to extremes, one could even argue that Kotlin on the JVM is nothing more than syntactic sugar.

Yes, and I subscribe to that opinion myself. :-) That said, Kotlin is clearly here to stay, so it makes sense for us to try to accomodate its users as good as we can.

I can just speak for myself when I say that (assuming there are no or negligible performance impacts with the noarg-plugin approach) I would have liked to know about having an alternative when starting off.

I think you raised some good points. If you want to introduce this into the Kotlin quickstart, I'll merge your PR. I appreciate you taking the time to go over this with me!

[greyhairredbear]

Hi, it's been a while, but I've finally been able to free up some time on my schedule to go into the discussed matters in more depth.

• First and foremost, I've run a benchmark using our (cargonexx) VRP model with what I suppose is a test dataset of medium complexity. The score calculation speed was pretty much the same (14.957/s for the manually written constructors and 15.424/s for the no-arg-plugin version). If you'd be interested in other benchmark statistics, I think I may provide the whole benchmark output here (might need to look into what I'd be sharing exactly, don't want to get into any legal trouble). Also, I could run further benchmarks with other datasets, but I'd expect very similar results for them.

• Regarding the synthetic constructor: I've decompiled the Kotlin bytecode of my TourPlan class that has a no-arg constructor generated and got the following. Seems like nothing unexpected/strange is happening here. Had to google the DefaultConstructorMarker type, sounded to me like it's a workaround/hack for Kotlin to know what the primary constructor is. This parameter gets always passed null as a value.

   public TourPlan(@NotNull String id, @NotNull List vehicles, @NotNull List actions, @NotNull TourPlanConstraintConfiguration constraintConfiguration) {
      Intrinsics.checkNotNullParameter(id, "id");
      Intrinsics.checkNotNullParameter(vehicles, "vehicles");
      Intrinsics.checkNotNullParameter(actions, "actions");
      Intrinsics.checkNotNullParameter(constraintConfiguration, "constraintConfiguration");
      super();
      this.id = id;
      this.vehicles = vehicles;
      this.actions = actions;
      this.constraintConfiguration = constraintConfiguration;
   }

   public TourPlan() {
   }

   @Metadata(
	// I've cut that part out, seemed like clutter [...]
   )
   public static final class Companion {
      private Companion() {
      }

      // $FF: synthetic method
      public Companion(DefaultConstructorMarker $constructor_marker) {
         this();
      }
   }

• I was indeed not able to set a breakpoint in the decompiled Kotlin bytecode.
• The stacktrace of an OutOfMemoryError caused by an invocation of the no-arg constructor using reflection looks like this:

Caused by: java.lang.OutOfMemoryError: Java heap space
	at java.base/jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(DirectConstructorHandleAccessor.java:67)
	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:500)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:484)
	at com.cargonexx.vehiclerouting.test.util.TestClass.test(TestClass.java:36)
...

while calling the no-arg constructor directly results in

Caused by: java.lang.OutOfMemoryError: Java heap space
	at com.cargonexx.vehiclerouting.test.util.TestClass.test(TestClass.java:36)

Bottom line
If this prompts more questions, I'm happy to answer them or investigate further.

In the context of the past discussion, I assume none of the points here are showstoppers for integrating the no-arg plugin into the kotlin quickstart, so I'll be quite happy to open a PR for that in the coming days :)

[triceo]

Thanks for the additional information. Pull requests are welcome!

[ge0ffrey]

Peter, yes, we welcome improvements that make it easier to use Timefold Solver from Kotlin :) Thanks for looking into this with Lukas.

[ge0ffrey]

We have added Kotlin code for all getting started guides:

[greyhairredbear]

I'm currently looking into adding JSpecify annotations to the ai.timefold.solver.core.api package (and its child packages). I've come across the @NullMarked annotation, which can be applied at the package level and indicates that all Type usages not marked with @Nullable are not nullable.

IMO, this might be a good way to avoid cluttering the code with too many annotations. However, when I started using this on interfaces in the api subpackages, I noticed IDE warnings in the implementations (e.g. DefaultSolverFactory) when these classes were not annotated correctly. Now, marking these methods correctly should probably not be an issue, but I fear using @NullMarked on these implementation classes might increase the chance that I miss some cases where types might be nullable.

I guess, it is preferable in the long run to use @NullMarked as much as possible, to avoid cluttering the code (as I mentioned).

I would like some opinions on how to proceed here (would be great if one of you could chime in@triceo @ge0ffrey) to save some effort and prevent errors. I suppose there are some options:

1. Annotate the ai.timefold.solver.core.api package and its subpackages with @NullMarked, annotate occurrences of nullable types accordingly. Annotate only overridden implementations in the impl package with @NullMarked and annotate nullable types accordingly.
2. Annotate both the api packages and the whole implementing classes with @NullMarked. This might increase the chance of missing nullable types in the implementation classes.
3. Annotate all type occurrences of the public API and the implementation with @Nullable/@NonNull annotations. Probably introduces too much clutter.

I guess I found my preferred solution when writing this (number 1), but it'd still be nice to get a second opinion here. :)

[triceo]

I don't really know the pros and cons of each approach and what the requirements for Timefold are here. Does it make a difference to you if you see org.jspecify imports vs ai.timefold imports for the annotations? Do you think it would make a difference regarding maintainability (only changing annotations on custom null-related annotations vs having to change all annotation occurrences)?

Personally, I don't mind; it's annotation for annotation. I think Spring does that, because they're in the pre-JSpecify situation, where they possibly may need to support multiple providers of these annotations. If we're standardizing on JSpecify, I don't see a problem in using it directly.

[greyhairredbear]

Another question regarding the annotations: Should the javadoc comments indicating never null and sometimes null be kept or be removed? They would probably be just duplication of the code anyways, but some @returns clauses would be left without any content if never null is removed there...

[triceo]

For Javadoc, I think it depends on what the rendered output actually looks like. Is the annotation in there somewhere? In other words - can I, as a user, have a way of knowing what to expect, without it having to be explicitly written there? Should the information disappear when we remove it from the Javadoc source, then I think it needs to be kept. This would also solve the return problem.

I'm not that deeply familiar with Javadoc, but I'm guessing empty @ return is fine if we decide to remove the never null comment?

[greyhairredbear]

Just fiddled around this in the IDE (so I was using the doc rendering from IntelliJ, but I guess it won't be much different when generating Javadoc with a different tool).

As I expected, the @Nullable and @NonNull annotations are kept in the method signature, so I guess it will be visible there. When keeping an empty @return, the Doc reads just "Returns: ", when also removing the empty @return, this part is omitted. Not sure what's preferable here.

[triceo]

If it doesn't generate Javadoc errors (the CI will catch those), then I say let's remove it. One less line which serves no purpose.

[mikepictor]

On this topic, there is one change that I am hoping may be easy for you to do with a timefold-kotlin library or something.

I tried to use an IntValueRange provider for work I was doing, but IntValueRange supplies Java integers, while my solution is using Kotlin Int, so I wasn't able to use it. If you made kotlin versions of some of these primitive utility tools that would be handy. I CAN code my app to use java.lang.Integer, but then I lose out on some Int related extensions and infixes. In the end I just use a List as my range provider.

I can live with having to use var and marking things nullable. I'd prefer not doing it (leads to lots of null checking variables that i know full well aren't null), but as OP wrote, it's just a matter of principle. They work fine as vars.

[greyhairredbear]

Hm, maybe that could be achieved with ValueRangeFactory? https://docs.timefold.ai/timefold-solver/latest/using-timefold-solver/modeling-planning-problems#valueRangeFactory

Haven't tried it myself, but I feel there might be a way to work around this issue...

[triceo]

Correct, a custom ValueRangeFactory can be implemented to provide you with this.
I never say never, but at this point in time, I don't see us providing Kotlin-specific functionality; the Solver core is not likely to have a dependency on Kotlin.