Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[11.x] Blade Component Loop Speed Improvement #51158

Merged
merged 5 commits into from
Apr 22, 2024

Conversation

lonnylot
Copy link
Contributor

Cache repeated processes that will not change creates a 7% performance improvement:

Before:

OPCache Enabled: Enabled
With caching: 206.043693

After:

OPCache Enabled: Enabled
With caching: 192.26193209091

Profiling methodology: #51141 (comment)

welcome.blade.php

@for($i = 0; $i < 1000; $i++)
    <x-avatar :name="'Taylor'" class="mt-4" />
@endfor

avatar.blade.php

<div>
    Hi! My name is {{ $name }}
</div>

routes/web.php

Route::get('/bench', function () {
    $a = array_map(
        fn () => Benchmark::measure(fn() => view('welcome')->render()),
        range(0, 10)
    );

    return 'OPCache Enabled: ' . (is_array(opcache_get_status()) ? 'Enabled' : 'Disabled') .
            '<br>With caching: ' . (array_sum($a) / count($a));
});

Comment on lines 264 to 271
if (isset($this->normalizedNames[$name])) {
return $this->normalizedNames[$name];
}


$this->normalizedNames[$name] = ViewName::normalize($name);

return $this->normalizedNames[$name];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This syntax should also be possible:

Suggested change
if (isset($this->normalizedNames[$name])) {
return $this->normalizedNames[$name];
}
$this->normalizedNames[$name] = ViewName::normalize($name);
return $this->normalizedNames[$name];
return $this->normalizedNames[$name] ??= ViewName::normalize($name);

@taylorotwell
Copy link
Member

taylorotwell commented Apr 22, 2024

Hi @lonnylot - could you fix the conflicts here?

Update: Think I got it.

@lonnylot lonnylot force-pushed the cache-view-factory-repeated-calls branch from 003d994 to be862c7 Compare April 22, 2024 19:16
@lonnylot
Copy link
Contributor Author

Hi @lonnylot - could you fix the conflicts here?

Update: Think I got it.

Pushed the fix before I saw the update

@lonnylot
Copy link
Contributor Author

👀 Slight mistake in the updated formatting that affects performance. Let me know if you want me to correct it or if you're taking care of it (don't want to cause conflicts)

@taylorotwell
Copy link
Member

@lonnylot you can fix.

@lonnylot
Copy link
Contributor Author

@taylorotwell Actually your update is better and I just adjusted the tests. I can come back later and do additional profiling to confirm the performance improvement still exists.

@taylorotwell
Copy link
Member

taylorotwell commented Apr 22, 2024

@lonnylot in my testing locally the performance improvements are still in place. 👍

For reference, rendering 20,000 components locally takes ~98ms before your change and ~90ms after. Taking average of 10 iterations per benchmark.

@taylorotwell taylorotwell merged commit 2b1ad99 into laravel:11.x Apr 22, 2024
27 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants