Only merge cached casts for accessed attribute

[ug-christoph]

This is a PoC for the issues discussed in #31778

One major performance hog that I noticed in my laravel jobs is the fact that all model attributes that have a cast and were already accessed are serialized everytime any attribute of the model is accessed. This happens in HasAttributes.php in the method mergeAttributesFromClassCasts.

Here is an example:

$user = User::first();
$someCastedAttribute = $user->some_casted_attribute;
$id = $user->id; // <-- this line triggers mergeAttributesFromClassCasts which causes a serialization of some_casted_attribute

This behavior is very expensive if the casted attributes are large objects. (e.g. an instantiated class from a large json column).

Let's assume $user->some_casted_attribute holds a class with 100kB of loaded data

for($i = 0; $i < 1000; $i++) {
    $id = $user->id;
}

This loop will cause 1000 calls to json_encode($this->some_casted_attribute) which effectively encodes a total of 100MB of data, even though some_casted_attribute is never used in the loop.

My PR includes a PoC that makes sure that only the attribute that is really accessed via getAttribute is serialized. This is optional behavior that I added to mergeAttributesFromClassCasts and it will behave as it used to for all other places in the codebase that access it.

The linked github discussion includes further suggestions how to reduce the calls for places like isDirty checks, but I decided to stick to the highest ROI change with the lowest chance of negative side effects in this PR.

Tests are passing and I cannot think of a way how this would break something, but I am happy to get confirmation that the approach is sound and that I am not overlooking some edge case or reason why we would actually want to serialize all casted attributes even when we just access id.

My benchmarks showed a huge improvement for my workloads. Before the change my code spent over 80% of the total execution time in mergeAttributesFromClassCasts. With the change this reduced <20% for most cases (still high, but definitely an improvement, that will save significant server costs). Disclaimer: My workload actually contains large json columns (10-100kB).

The screenshot lists some of my jobs which have a high execution time % spent in mergeAttributesFromClassCasts with the time spent before and after this proposed change.

[marius-ciclistu]

This should be directed towards 13.x/master because it contains breaking changes in the function definitions.

[ug-christoph]

The affected tests are quite brittle (they do an assertions on the exact call count to a method). The PRs goal is to directly reduce those calls. The test is a good example of where the optimization helps. In these tests $subject->id is called which causes mergeAttributesFromCachedCasts to run for the encrypted/casted columns (secret_array, secret_collection). This is now expectedly not happening anymore and I have therefore just reduced the call count in the assertion by one to reflect this improvement

[ug-christoph]

This should be directed towards 13.x/master because it contains breaking changes in the function definitions.

Thank you for the comment. I now changed the code so the original signatures remain the same

[ug-christoph]

Instead of calling

return $this->getAttributes()[$key] ?? null;

I do exactly what getAttributes does, but instead of calling mergeAttributesFromCachedCasts
I call the new
mergeAttributeFromCachedCasts($key) so it only does it for the attribute I am currently trying to get

[ug-christoph]

this is the same as mergeAttributesFromCachedCasts, but it uses the new extracted methods mergeAttributeFromClassCasts and mergeAttributeFromAttributeCasts that allow to do it for just one attribute

[ug-christoph]

I extracted the code that was inside this loop to a separate method, so I can use it here and also independently for getAttribute

[ug-christoph]

I extracted the code that was inside this loop to a separate method, so I can use it here and also independently for getAttribute

[taylorotwell]

How does this affect situations where the value of the attribute you are trying to access is affected by a mutator which depends on an unrelated cached class cast?

[marius-ciclistu]

@taylorotwell That is similar with the question no one replied to here #31778 (reply in thread)

So there are cases like that. Can you please give an example?

Update:

If the get Mutator uses $this->attributes this can happen

$model = User::query()->firstOrFail();

$carbon = $model->date_time_carbon_casted;

$carbon->addDay();

echo $model->col_with_get_mutator_that_depends_on_date_time_carbon_casted;

// will print 'value date time' without the added day.

In current scenario getAttributeValue will call transform on getAttributeFromArray that will call getAttributes that will merge all casted objects to prevent the above.

A workaround for performance is to use in the mutator $this->getAttributeFromArray if you have object cast for that column or getAttributeValue if you want the related value transformed.

[ug-christoph]

@taylorotwell I added a new test to cover the mutator cast dependency:
\Illuminate\Tests\Integration\Database\DatabaseEloquentModelCustomCastingTest::testMutatorCanDependOnAnotherCastedAttribute

I am not sure if this covers the exact case that you described. Please provide a code example of the situation you are concerned about if the test does not already cover it.