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] Add component view instance caching #51152

Closed

Conversation

amir9480
Copy link
Contributor

This PR will add a view instance caching support to components and prevent creating new view instances on each render resulting in increasing +20% performance rendering components.

Benchmarks:
Before the view instance caching:
image

After the view instance caching:
image

@taylorotwell
Copy link
Member

With the merge of other recent performance improvement PRs I'm actually not seeing any meaningful performance benefit from this PR anymore. Can you re-run your tests with latest code in 11.x? Mark as ready for review if you want me to take another look or after you comment.

@taylorotwell taylorotwell marked this pull request as draft April 22, 2024 21:07
@amir9480
Copy link
Contributor Author

amir9480 commented Apr 22, 2024

@taylorotwell

This PR is not needed anymore.

UPDATE:
I'm not sure how, but this PR without View::composer doesn't have any effect and if we use this PR with View::composer still there is performance improvement.

Before:
image

After:
image

My closed PR (#51141) also can improve performance.

Without in-memory cache support for Blade:
image

With in-memory cache support for Blade:
image

Note: The first and second test runs with different codes for testing.

@amir9480 amir9480 marked this pull request as ready for review April 22, 2024 21:39
@amir9480
Copy link
Contributor Author

@taylorotwell
This PR can cause side effect issues as it caches view data.

web.php:

Route::get('/', function () {
    return view('home');
});

home.blade.php:

<x-sample param1="Hello" />
<hr />
<x-sample param1="Hello" param2="World" />
<hr />
<x-sample param1="Hello" />

sample.blade.php:

@if (isset($param1))
    <p>Param 1: {{ $param1 }}</p>
@else
    <p>Param 1 is not set</p>
@endif

@if (isset($param2))
    <p>Param 2: {{ $param2 }}</p>
@else
    <p>Param 2 is not set</p>
@endif

Output:
image

@amir9480
Copy link
Contributor Author

Fixed the reuse data issue and this PR still can improve performance while using View::compose.

Without view instance caching:
image

With view instance caching:
image

@amir9480 amir9480 force-pushed the add_component_view_instance_cache branch from ecf48cd to d2153d6 Compare April 23, 2024 08:21
@amir9480 amir9480 force-pushed the add_component_view_instance_cache branch from d2153d6 to 02c67f0 Compare April 23, 2024 08:41
@amir9480 amir9480 closed this Apr 23, 2024
@amir9480
Copy link
Contributor Author

Causing other side effects, closing the PR.

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.

2 participants