Feat: support for avoiding service loaders

[marinier]

Is your feature request related to a problem? Please describe.

I'm running Timefold in an environment where I can't use a ServiceLoader. (This is on Android, and while service loaders work on Android in general, in my particular case it doesn't work for Timefold and I don't have access to the code that's causing the problem. Besides, I don't think wanting to avoid service loaders is necessarily an Android-specific issue.)

The place where there's really a problem is in ScoreDirectorFactoryFactory::decideMultipleScoreDirectorFactories which uses a ServiceLoader to discover the different score director factory implementations and then picks one. In my case I just want to use Bavet, so no discovery is actually needed.

Hacky workaround

My plan was to make my own ScoreDirectorFactoryFactory that overrode this method to just return a BavetScoreDirectorFactory, like this:

public class MyScoreDirectorFactoryFactory<Solution_, Score_ extends Score<Score_>> extends ScoreDirectorFactoryFactory<Solution_, Score_> {

    protected ScoreDirectorFactoryConfig config;
    
    public MyScoreDirectorFactoryFactory(ScoreDirectorFactoryConfig config) {
        super(config);
        this.config = config;
    }
    
    @Override
    protected AbstractScoreDirectorFactory<Solution_, Score_> decideMultipleScoreDirectorFactories(
            ClassLoader classLoader, SolutionDescriptor<Solution_> solutionDescriptor, EnvironmentMode environmentMode) {
        
        ScoreDirectorFactoryService<Solution_, Score_> service = new BavetConstraintStreamScoreDirectorFactoryService<>();
        Supplier<AbstractScoreDirectorFactory<Solution_, Score_>> supplier = service.buildScoreDirectorFactory(classLoader, solutionDescriptor, this.config, environmentMode);
        return supplier.get();
    }

}

This went mostly smoothly except that the config member in the base class is private, so I had to make my own member for it.

The next step was to subclass DefaultSolverFactory and override the buildScoreDirectorFactory method to use MyScoreDirectorFactoryFactory. Unfortunately, this proved impossible. The DefaultSolverFactory class is final so it can't be subclassed, and even if it wasn't the buildScoreDirectorFactory is private, so it can't be overridden (although I could have worked around that by changing the constructor in a subclass).

I don't like it, but I made a class MySolverFactory that is a complete copy of DefaultSolverFactory except for the change to the buildScoreDirectorFactory.

I then had to do similar things to use Joiners without using ServiceLoader -- because it is also final and static, I had to make MyJoiners which manually loads DefaultJoinerService (again a complete copy of Joiners, which isn't great).

And that's not the only problem -- I was using SolutionManager to print out debugging information about the solution, like this:

LOG.debug(SolutionManager.create(solverFactory).explain(solution).getSummary());

But the create method is hard-coded to make a DefaultSolutionManager, which is in turn hard-coded to expect a DefaultSolverFactory, so this doesn't work with MySolverFactory. DefaultSolutionManager is also final, so I tried to follow the same pattern as above and create a MySolutionManager class by copying DefaultSolutionManager and changing it to work with MySolverFactory. Unfortunately, the code references the Fitter class, which is not public. So I've set aside the ability to print debugging information for now.

In the end it works, which is fine for the short term, but not a good long-term solution.

Describe the solution you'd like

I'm not sure what the goals of the existing code are (why are the classes final in the first place?), so it's hard for me to know what the right solution is. Some possibilities include:

• Solution 1: Make the various classes more friendly to extension. That is, don't make them final, and make the various members and methods protected instead of private. In places like DefaultSolutionManager's constructor don't hard-code a cast to DefaultSolverFactory (or maybe this is ok if DefaultSolverFactory can be subclassed). Joiners can't be subclassed because it's all static methods, but perhaps it could be given a static method to set which JoinerService to use in order to bypass the ServiceLoader.

• Solution 2: Add versions of some methods that allow the specific classes to be used to be specified. E.g., maybe DefaultSolverFactory could get a new constructor that takes an instance of InnerScoreDirectorFactory to use so it doesn't have to be discovered. JoinerService could use the same approach as solution 1 in this case.

• Solution 3: In the config allow the specific implementation classes to be specified, and if they are just load them directly instead of using a service loader. This might go along with solution 2.

• Solution 4: If loading from a service loader fails, fallback to a default implementation instead of failing (possibly making this fallback behavior configurable). This is not as flexible as solution 3, but if 95% of users are using the same services anyway (e.g., BavetConstraintStreamScoreDirectorFactoryService + DefaultJoinerService), then this might be a reasonable thing to do and it's probably much easier than the above solutions.

Describe alternatives you've considered

The workaround I described above works, but it's not going to be maintainable long-term. That is, I don't want to have to update my versions of the classes every time the timefold versions change. It also just feels wrong, like I'm doing something that you tried to prevent for some reason. I just really want there to be a right way to do this.

Additional context
N/A

[triceo]

Hello @marinier. First of all, thank you for taking the time to submit this feature request; I appreciate that you clearly laid out what you want and what you attempted to get it done. There is a lot to unpack here, I'll take it in parts.

Service loader for Constraint Streams/Joiners

The reason why CS is loaded via a service loader is purely historical. There once used to be different implementations of CS, and this was the best way to make them independent. There is no such need anymore and so we may eventually remove the service loaders from this part of the codebase entirely; it's not high on the priority list, but it is also not too hard, so it may happen sooner rather than later.

Service loaders in general

Even if/when we remove the service loaders from CS, they will still remain elsewhere in the codebase. For example, Enterprise Edition of the solver relies on service loaders heavily for its plug-and-play nature. We are unlikely to ever change this aspect. Which brings me to a larger question - what is wrong with service loaders?

We occasionally hear that users want to avoid service loaders. Not too often, but we do hear it. Unfortunately, we never managed to find a specific reason for wanting to avoid service loaders, other than user preference.

To the best of my knowledge, service loaders are:

• Used throughout the JDK, so it is a proven and stable technology.
• Supported in Quarkus and Spring.
• Supported in Native images.
• Supported on Android.

Perhaps you will be able to add the missing piece to the puzzle for me. What are we missing? What is your reason to avoid service loaders?

Finality of classes

This is a deliberate decision. It states our intent not to have these classes extended. We are very careful about our backwards compatibility, we go to great lengths not to break people as we evolve the solver. Exposing our internals for extension adds a layer of API surface we are not prepared to open.

You might argue that this prevents users such as yourself from adapting the solver to your needs. And you would be right. But it also has another effect - it brought you to talk to us, explain your use case and we may now together figure out an actual solution as opposed to "a hacky workaround" as you put it.

[marinier]

Thank you for your quick response! I had a breakthrough today that I think means there's a much simpler solution.

The reason to avoid service loaders in my specific case is that it wasn't working. My project is a plugin for another android application that I have no control over, and it turns out that it loaded plugins using a thread that had a context class loader that was unable to find the timefold services file. However, the normal Android classloader is able to load that file just fine. So rather than avoid the classloader entirely, I just need to be able to specify which classloader to use.

And this almost works in timefold -- I can call SolverConfig::setClassLoader to specify a classloader, and this classloader gets passed into ScoreDirectorFactoryFactory::decideMultipleScoreDirectorFactories. Unfortunately, it doesn't get passed into the call to ServiceLoader.load on line 69. So, using my hacky classes from yesterday I changed this method to be exactly the same as it is in timefold except I pass in the classloader like this:

        ServiceLoader<ScoreDirectorFactoryService> scoreDirectorFactoryServiceLoader =
                ServiceLoader.load(ScoreDirectorFactoryService.class, classLoader);

And now it works correctly! I'm hoping that passing in a custom class loader to the service loader is much less controversial than bypassing the service loader altogether.

As a side note, it doesn't look like there's a .withClassLoader method on SolverConfig. Not a big deal, but would be nice for consistency.

So to summarize, the changes I'd like are:

• Modify ScoreDirectorFactoryFactory::decideMultipleScoreDirectorFactories to pass the classloader argument to the ServiceLoader.load call
• Add SolverConfig::withClassLoader

[marinier]

I have added separate issues for the above requests. Thinking more about this issue, however, I think I'm realizing what the actual issue with service loaders is. The real issue is that, for the common case where you know exactly what you want and don't actually need to discover it at runtime, it adds a layer of complexity that makes debugging much more difficult. I have lost countless hours, not just on this project but on other projects that use service loaders for other purposes, trying to figure out why service loaders aren't working. It usually ends up being some kind of classpath or module path issue (e.g., somehow the desired class doesn't get packaged in a jar, or it has dependencies that don't get packaged, or a module doesn't opens it, etc.), but figuring that out takes a lot of time. On this project it turned out that the default class loader wasn't able to load the classes, even though they were on the classpath. All of this could have been avoided if I could have just passed the desired class directly instead of having service loaders discover it.

I'm not suggesting that service loaders be dropped (there are definitely cases where you actually want discovery), I'm simply suggesting that for those cases where discovery isn't needed that it be much easier to bypass the mechanism.

I can think of a couple ways to do this:

• Extend ServiceConfig to allow the specific classes to use to be specified (much like the class loader can be specified). Not the names of the classes but the actual class instances. If the config contains this information, then it is used instead of the service loader.
• Make it easier to create a custom ScoreDirectorFactoryFactory so the logic of decideMultipleScoreDirectorFactories can be customized. This is currently hard because DefaultSolutionManager is final, and that needs to be modified to tell the system to use the custom ScoreDirectorFactoryFactory. I understand that this is to preserve backwards compatibility. I don't know the details of exactly how making these final does that, but one possibility would be to move the logic to an intermediate abstract class, (e.g., AbstractDefaultSolutionManager). Then have DefaultSolutionManager subclass that -- it can still be final, but there won't really be any logic in it. But if someone wants to customize the class they can create their own subclass of AbstractDefaultSolutionManager, which won't be final, and then make whatever changes they want. This ensures that DefaultSolutionManager remains final as desired, while still allowing for extensibility. Personally, I don't like this approach as a solution just for avoiding service loaders as it makes everything customizable, but from the more general perspective of general extensibility I like it a lot, as there may be many reasons to want to customize something that have nothing to do with service loaders.