Call __init__() from request thread pool

[deliahu]

@RobertLucian @vishalbollu what do you think about this? Are there any issues you can think of with this approach?

[vishalbollu]

This looks like an interesting idea. I have a few points:

If each thread runs its own predictor constructor, does this mean that each thread loads a model into memory separately? At that point, it may be safer to use processes instead of threads. If that is the case, we can also only initialize the threadpool if the thread is of size 1 (to avoid OOM). At least this way, running TensorFlow sessions should work by default when threads_per_worker is set to 1.

Another consideration, which isn't mutually exclusive to the above, is to import the threadpool from serve.py in the predictor implementation and run initializations in the predictor's constructor. We can have a guide showcasing this pattern if it works (and I am not sure that it does work).

[RobertLucian]

Cool that it works! I think this is a good idea and solves the immediate issue that TensorFlow presents (aka, having to reinitialize things when moving thread to thread).

The bad part about this is if the user is not aware of this, they can potentially experience that TF bug/issue/limitation when threads_per_worker > 1 and this can generally lead to a confusing experience with Cortex - it could take some time until they ask for support from us. It can look like the Cortex functionality is fragmented as opposed to just keeping it not working regardless of how many threads there are per worker, even though this is a TF limitation.

I'm thinking we could probably mitigate this confusion by adding a big warning sign in the docs mentioning the behavioral difference when threads_per_worker > 1.

Also, I'm wondering how much production deployments could benefit from it, as those would probably have threads_per_worker set to something higher than 1 and thus will require a different implementation of the predict method. Simple deployments with threads_per_worker set to 1 and workers_per_replica >= 1 will definitely benefit from this.

I don't see a way of initializing the constructor in each thread such that data duplication is avoided, so maybe this is the closest we can get to.

[RobertLucian]

LGTM!