Alternative to fix RD-14971: remove inferrer threads and make inferre… #528
Conversation
| @@ -33,11 +33,14 @@ class ProgramContext( | |||
| private val stageCompilerCache = new mutable.HashMap[SnapiProgram, Either[ErrorCompilerMessage, SnapiValue]] | |||
|
|
|||
| def infer( | |||
| inferrerProperties: InferrerInput | |||
| inferrerProperties: InferrerInput, | |||
| mandatoryArgs: Seq[Arg], | |||
There was a problem hiding this comment.
This is not enough. It needs to distinguish csv from json calls and so forth. So it needs at least one extra 'string ' argument with that.
| * @param varArgs the varargs | ||
| * @param inferrerInput the inferrer input | ||
| */ | ||
| private case class CompilerContextInferrerKey( |
There was a problem hiding this comment.
It could be a real class to not confuse anyone with the usual way a case class is equal to another one.
| ) { | ||
| // Override equals and hashCode to consider only the arguments, not the properties | ||
| override def equals(other: Any): Boolean = other match { | ||
| case that: CompilerContextInferrerKey => this.mandatoryArgs == that.mandatoryArgs && |
There was a problem hiding this comment.
When the first arg is a location (which it is in general, even when we pass a URL, it's turned into a location), the object we get is a SnapiLocationValue, which contains a Location. Which is the class that doesn't have a working comparison, the one used in InferrerInput that makes it not comparable.
| return new Value(compilerContext, inferrer); | ||
| }); | ||
| public InferrerService getInferrer(RawUid user, RawSettings rawSettings) { | ||
| return inferrerServiceCache.computeIfAbsent(user, k -> InferrerServiceProvider.apply(rawSettings)); |
There was a problem hiding this comment.
This is creating an inferrer with specific rawSettings, and store it for key user. Later even if passing other rawSettings, we'll find the one stored for the user key and return it (the one that has former settings).
|
|
||
| private val inferrerThreadPool = Executors.newCachedThreadPool(RawUtils.newThreadFactory("inferrer-thread")) | ||
|
|
||
| private val inferCache: LoadingCache[InferrerInput, Either[String, InferrerOutput]] = CacheBuilder |
There was a problem hiding this comment.
The inference cache is in the CompilerContext now. And I remember that's the one which is created once and for all per user, right? (Maybe a second one is created for nested staged compilations, but that wouldn't another story). That ensures the cache never goes away.
The thread pool being a CachedThreadPool lets it grow ad infinitum but threads are eventually deleted (one minute by default). Under normal behavior we should see the number of inferrer threads being stable (proportional to the number of active users basically).
Remove threads from the inferrer. Therefore, it is no longer a RawService/does not need to be stopped.
Since the inferrer is not "per user", change the code to instantiate it once, or rather, once per "RawSettings"
The thread pool/caching is now part of the CompilerContext. It considers the language nodes as cache keys.