Yii2: Enhancement of behavior: performance optimization by ie. request

[dynasource]

My projects are high dependant on behaviors. When playing with xdebug-profile, I came to the conclusion that behaviors come at cost of performance (of course), but it also showed me that behaviors() in the example of timestampbehavior (http://www.yiiframework.com/doc-2.0/guide-concept-behaviors.html#using-timestamp-behavior) is not optimal in every usecase.

For example. The timestamp & blamable behavior is only relevent in a POST request. But the behaviors are also connected in a GET request, which means the attachment is useless. In one of my haviest index pages, this shows a performance gain of 1.5%:

• intially: 9.419 MB
• post only: 9.283 MB

A save of 0.136 MB when both Blameble- & TimestampBehavior are ignored during GET requests. When doing a xdebug-profile, this performance gain is also spectacular.

To conclude. I would suggest to enhance Behaviors on this matter.

A solution shoud be a mechanisme declaring if it should be attached or not:

• a internal function in Behavior
• a boolean config option in Behavior

[samdark]

There's a under discussion plan to drop behaviors in favor of traits + events in 2.1 but for 2.0 it sounds like a good thing to do.

[dynasource]

can you point me to this discussion? https://github.com/yiisoft/yii2/issues/1053?

[samdark]

Yes. That was the start of it. We've further experimented and later @creocoder provided a good way to attach/detach via adding/removing event handlers.

[samdark]

Overall topics to look for 2.1 are here: https://github.com/yiisoft/yii2/wiki/Ideas-for-2.1

[klimov-paul]

The timestamp & blamable behavior is only relevent in a POST request. But the behaviors are also connected in a GET request, which means the attachment is useless.

For some behaviors this is so indeed, but in general it is not. For example:
https://github.com/yii2tech/ar-variation
https://github.com/yii2tech/ar-role

A solution shoud be a mechanisme declaring if it should be attached or not

You may always do something like:

public function behaviors()
{
    if (Yii::$app->request->method === 'get') {
        return [];
    }
    return [
        'timestampBehavior' => [
                'class' => TimestampBehavior::className(),
                'createdAtAttribute' => 'createdAt',
                'updatedAtAttribute' => 'updatedAt',
            ],
    ];
}

can you point me to this discussion?

1053 #1042

Also, there is an extension, which allows event handling via traits:
https://github.com/yii2tech/behavior-trait

[samdark]

Probably the way @klimov-paul is using to attach TimestampBehavior should be advertised in docs.

[dynasource]

Yes. I am using this procedure. But it lacks checking on if request exists. This must be included to prevent errors in tests or console.

[samdark]

public function behaviors()
{
    $behaviors = [];
    if (!Yii::$app->has('request') || !Yii::$app->request->isGet) {
        $behaviors['timestampBehavior'] = [
                'class' => TimestampBehavior::className(),
                'createdAtAttribute' => 'createdAt',
                'updatedAtAttribute' => 'updatedAt',
        ],
    }
    return $behaviors;
}

But it has one issue. Request data is queried inside a model which isn't quite right because the only place request should be accessed is controller.

[dynasource]

True. The usecase does show room for improvement of AR. Other options:

• add a variable in AR defining AR::load() has been executed (instead of checking a request)
• only attach behaviors at afterSave

[samdark]

add a variable in AR defining AR::load() has been executed (instead of checking a request)

Yes, that could be a good idea.

only attach behaviors at afterSave

There are other behavior types so it won't work.

[dynasource]

There are other behavior types so it won't work.

I mean within this specific context. Why attaching a behavior if it won't be used. Depending on if behaviors are dropped in 2.1, this would be an area to put time in to improve.

[klimov-paul]

Actually behaviors are attached only on magic method call or trigger() invocation.
Such problem should not even exist at all, however ActiveRecord triggers 'init' event just after instantiation:
https://github.com/yiisoft/yii2/blob/master/framework/db/BaseActiveRecord.php#L848

This make all behaviors being instantiated and attached.

Originally there was no such event, it was added by commit:
6dd495b65f81e4c3f9efafe43d22509087887682

You may attempt to override init() method for ActiveRecord to get rid off trigger('init'), then it will be no trouble at all.

[dynasource]

to continue this conversation. Isnt it time to develop a solution for this issue. Most people use these default behaviors and the performance gain is too significant, especially when instantiating multiple ActiveRecords. As many of us have concluded many times, setting events is costly. And these AttributeBehaviors are setting events at i.e. index-actions for no reason. This is something to be fixed.

Would it be an idea to put conditions in these behaviors itself to check on Yii::$app->request. Default should be setting these events only at isPost (as this will cover 99% of the usecases) but there should be an option to ignore this condition for alternative usecases.

[samdark]

As I've already noted, it sounds wrong to put a check for request into the model since it could be used in web, console and even request-less environment.

[dynasource]

there are already checks on environment specific variables, like $app->user https://github.com/yiisoft/yii2/blob/master/framework/behaviors/BlameableBehavior.php#L100

To put it this way. We are talking about a 'behavior'.
This means particular 'way of behaving' in a certain 'scenario'.

A timestamp behavior is only required in a specific scenario (when saving and 99% in a POST scenario). Same applies for blameables. It is logical to extend these behaviors with checks on the specific scenario's. This could be a check on a user, a request or another app component.

The current implementation is not good enough on this matter. It is chosen to load at all time, which is not required in practice.

[samdark]

A timestamp behavior is only required in a specific scenario (when saving and 99% in a POST scenario).

Nope. I'm using it from console most of the time and there's no POST at all.

[samdark]

I agree that a check may be good but checking for request type isn't a good choice.

[dynasource]

I'm using it from console
Is it right for me to assume that most of Yii's users, and specifically using these kind of behaviors, are using it in a web environment? If that is so, the majority will benefit from checking on a request.

I would suggest fix this issue for the majority but to keep the solution as simple as possible. A solution like this is not 'sounding wrong' with respect to the logic behind hiding implementation if its not required.

   public function init(){
        if($this->isYetRequired()){

        }
   }


    public $requiredWhenGetRequest = false;
    private function isYetRequired()
    {
        if (!YII_CONSOLE && !$this->requiredWhenGetRequest && Yii::$app->has('request') && Yii::$app->request->isGet) {
            return false;
        }
        return true;

    }

[dynasource]

perhabs it is possible to put this logic into an interface in which Behavior.php checks against cases in which the behavior should not be applied.

[samdark]

Thinking more about it, I'm against this. Imagine you're writing stats using a model and you're incrementing it when the page loads. That's GET request but you need to save a model. If we'll add such a check then:

1. BC will be broken.
2. It will be way to hard to figure out why magically time isn't set in _some_ cases.

[dynasource]

even more illogical is the following scenario:

• you have a listview serving 100 pictures
• all of the picture have BlameableBehavior + TimestampBehavior (and perhabs even more)
• at least 2 x 100 = 200 unnecessary events are set

[dynasource]

I must admit that Im a bit surprised by the 'contra-thinking' at this subject. If were talking about the key values of Yii, one just has to visit the frontpage of http://www.yiiframework.com/ to see that 'Fast' is the firstly name key-value of Yii. If that really is an important value, we should do more to prove that, on regular base.

[rob006]

If performance is so important, you could create inherited model for read only use, that overrides behaviors list and removes unnecessary ones.

[samdark]

Is it possible to do that at the model level in behaviors() method?

[samdark]

For example, you can introduce readonly scenario and exclude behaviors based on that.

[dynasource]

using Template method for solving this issue it not optimal. If you have 20 different AR's models, you should copy this behavior 20 times.

Apart from that, we should not fix this issue with 'tricks'. It a fundamental issue IMHO, proven by usecase https://github.com/yiisoft/yii2/issues/9667#issuecomment-252606608. It should be fixed at its root.

As this is issue is primarily based on the the setting of events inside Behavior.php, I would suggest to fix it over there. The setting of events should be cancelled in certain scenario's. Still, other methods in this behavior should continue to work.

[dynasource]

For instance in case of Blameable, it could also contain a relation/method like: getCreator()

[samdark]

If it can be solved reliably, I'm OK doing that. The issue is that we can't reliably detect if the model will be saved or not automatically.

[samdark]

Probably introducing a built-in scenario named readonly could be a good solution.

[dynasource]

I will prepare a PR for this

[rob006]

Apart from that, we should not fix this issue with 'tricks'.

Separate read-only model is not a trick - if you're expecting that your model will have different behavior in certain situations, separate model class is cleanest way to do that.

@samdark

Probably introducing a built-in scenario named readonly could be a good solution.

Is it possible to set scenario for ActiveRecord::findAll() result?

[dynasource]

Separate read-only model is not a trick

never said it was. Template method is a trick.

[samdark]

@rob006 yes via overriding corresponding method.