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

Memory leak inside NamespacedItemResolver::parsed() cache when Octane is used #415

Closed
zarikx opened this issue Nov 4, 2021 · 3 comments · Fixed by #416
Closed

Memory leak inside NamespacedItemResolver::parsed() cache when Octane is used #415

zarikx opened this issue Nov 4, 2021 · 3 comments · Fixed by #416
Assignees
Labels

Comments

@zarikx
Copy link

zarikx commented Nov 4, 2021

  • Laravel Version: 8.69.0
  • PHP Version: 8.0.12
  • Database Driver & Version: N/A

Description:

There are some caching mechanism inside \Illuminate\Support\NamespacedItemResolver::parseKey() method.
But this cache is never flushed when running Laravel Octane. This leads to some memory leak when many validation errors are being processed. It’s pretty easy to distinguish when attribute values are big enough.

Steps To Reproduce:

Using fresh install of Laravel Octane, you do something like this:

use Illuminate\Support\Facades\Route;
use Illuminate\Support\Facades\Validator;
use Illuminate\Support\Str;
use Illuminate\Validation\ValidationException;

Route::get('/', static function () {
    $data = ['name' => Str::random(65536)];

    $validator = Validator::make($data, [
        'name' => 'string|max:50',
    ]);

    try {
        $validator->validate();
    } catch (ValidationException) {
        info('exception', [
            'memory' => round(memory_get_usage() / 1024 / 1024, 4),
        ]);

        return 'error';
    }

    return 'ok';
});

Inside logs you can see that memory usage increases with every request:

[2021-11-04 16:19:30] local.INFO: exception {"memory":17.0533} 
[2021-11-04 16:19:30] local.INFO: exception {"memory":17.4827} 
[2021-11-04 16:19:31] local.INFO: exception {"memory":17.6159} 
[2021-11-04 16:19:32] local.INFO: exception {"memory":17.7491} 
[2021-11-04 16:19:32] local.INFO: exception {"memory":17.8826} 
[2021-11-04 16:19:32] local.INFO: exception {"memory":18.0158} 
[2021-11-04 16:19:33] local.INFO: exception {"memory":18.149} 
[2021-11-04 16:19:33] local.INFO: exception {"memory":18.2822} 
[2021-11-04 16:19:34] local.INFO: exception {"memory":18.4155} 
[2021-11-04 16:19:34] local.INFO: exception {"memory":18.5487

I did some performance testing without this cache, and I haven't seen any improvement, only within a margin of error.
Maybe we can consider deleting this caching to not overthink this issue?

@driesvints driesvints transferred this issue from laravel/framework Nov 4, 2021
@nunomaduro
Copy link
Member

Working on it...

@foremtehan
Copy link
Contributor

@zarikx thank you for reporting this important issue, i use a lot translator in my app

@nunomaduro
Copy link
Member

This issue will be fixed on the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants