Flask: Don't create View instances for each request

[davidism]

View.as_view creates a function that instantiates the class every request. This seems inefficient for no real gain.

class DeleteView(View):
    methods = ['DELETE']

    # this only needs to happen once
    def __init__(self, model):
        self.model = model
        # imagine if we did something expensive here
        # like generating a form based on the model

The only argument I could think of for instantiating every request is if the constructor does something different based on the request. That seems like bad design, but any use that I can think of for that could be accomplished with properties instead.

Yes, there are ways to make it less expensive, like generating more ahead of time and passing more arguments, but that is not intuitive, and makes the class just a more complicated view factory.

[ThiefMaster]

I don't see how this can safely be changed without risking to break people's code if they store stuff on self e.g. because they call other methods in the same class and pass some thing via attributes instead of arguments.

Besides that, instantiating a class is quite cheap...

[davidism]

It's not cheap if you do something expensive in the constructor, which until I dug into the implementation was my intuition of how to use them.

[davidism]

Storing things on self from other methods is equivalent to storing globals, which would break for the usual reasons.

[ThiefMaster]

only if an instance is reused for more than one request. as long as the instance is request-scoped, storing stuff on self is like setting a local variable in a normal view function

[davidism]

Ah, I see. I wonder how common that is? Or at least how many are written in such a way that they would break with a single instance.

I'd rather change the behavior and put a big warning in the changelog / docs that they should store data on g.

[davidism]

Maybe add a every_request=True arg to as_view, and issue a deprecation warning that it will change to False in the future.

[ThiefMaster]

If we add this I would rather go for this thing to always be configurable or even better add a SingletonView base class that is instantiated once. Like this you can pick what suits you more instead of having

[davidism]

I'll have to think on how we could do SingletonView without copying the View.as_view code, which would be harder to maintain. But I'm ok with it in principal, assuming we change the docs to push it as the normal choice and View as a more advanced choice.

[jeffwidman]

Isn't storing stuff on self already discouraged in favor of storing stuff on g?

Personally, I'd rather move people towards a best practice and worry less about breaking their code, as long as we do it in a major version bump (and the upcoming 1.0 feels like an ideal time).

[ThiefMaster]

g is something to use across the code (e.g. for stuff like the current user). Putting application data into g that is used within a single class seems pretty ugly to me.

[justanr]

@davidism

I'll have to think on how we could do SingletonView without copying the View.as_view code, which would be harder to maintain. But I'm ok with it in principal, assuming we change the docs to push it as the normal choice and View as a more advanced choice.

I'd be okay with this, as View allows you to do stuff like request based DI into a view (e.g. make this instance if X otherwise make that one), plus self is cheap storage if the view class has multiple methods.

This call here can be modified in the SingletonView to create the instance and store the reference onto the class, something like:

class SingletonView(View):
    instances = {}

    @classmethod
    def as_view(cls, name, *class_args, **cls_kwargs):
        view = super().as_view(cls, name, *class_args, **kwargs)

        def singleton_view(*cls_args, **cls_kwargs):
            view_inst = cls.instances.get(name)
             if not view_inst:
                 view_inst = cls.instances[name] = cls(*cls_args, **cls_kwargs)
             return view_inst

        view.view_cls = singleton_view
        return view

That's just a rough draft idea, but something along those lines would not create a new instance for each request and allow the view class to be created for multiple end points.

[fcracker79]

What about object pooling?
This way no such code change will be required to benefit from such an improvement.

[davidism]

You'll need to explain what you mean.

[mitsuhiko]

This was done intentionally because people can stuff state on self between functions. It matches the behavior in Django.

[davidism]

We could probably do something with __setattr__, similar to how the Flask object prevents setup methods after handling requests. From my experience on Stack Overflow, the users who would use globals will find a way and insist it's ok anyway.

[mitsuhiko]

@davidism thing is that I have used the fact that self is fresh from request to request myself plenty of times. The allocation of the class instance is insignificant compared to what else happens on request dispatching.

[mitsuhiko]

Changing this now will break a lot of code since this is also explicitly documented behavior:

The way this works is that whenever the request is dispatched a new instance of the class is created and the dispatch_request() method is called with the parameters from the URL rule. The class itself is instantiated with the parameters passed to the as_view() function.

[davidism]

@mitsuhiko would you be ok with a new SingletonView subclass?

[mitsuhiko]

@davidism i could imagine a lifetime attribute on the view that the generic as_view function internally references. It could be 'request' which means it lives for the single request only, 'application' which binds it to the application instance or 'global' which makes it an actual singleton that then cannot have state.

Generally the only values I think make sense are 'request' and 'application' though.