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

[8.x] Increase reserved memory for error handling #42646

Merged
merged 3 commits into from
Jun 3, 2022
Merged

[8.x] Increase reserved memory for error handling #42646

merged 3 commits into from
Jun 3, 2022

Conversation

spawnia
Copy link
Contributor

@spawnia spawnia commented Jun 3, 2022

Follow up to #42630

The error handler should be able to report exceptions
arising from exceeding the PHP memory limit. Because
of changes made to error handling, the previously
configured value is no longer sufficiently large.

A similar change was also done in Symfony a while ago,
see symfony/symfony#44327.

I used the following artisan command to determine the amount
of memory required for error handling, using a reporter that
simply writes to a file:

<?php declare(strict_types=1);

namespace App\Console\Commands;

use Illuminate\Console\Command;

final class MeasureHandlerMemory extends Command
{
    protected $signature = 'measure-handler-memory';

    private int $peak;

    public function handle(): void
    {
        $this->peak = memory_get_peak_usage();

        trigger_error('', E_USER_ERROR);
    }

    public function __destruct()
    {
        $used = memory_get_peak_usage() - $this->peak;
        echo "error handling used: " . $used . "\n";

        $before = memory_get_usage();
        $times = 10240;
        $reserve = str_repeat('x', $times);
        $after = memory_get_usage() - $before;
        echo 'repeat times ' . $times . ' reserves: ' . $after . "\n";

        $ratio = $after / $times;
        echo 'ratio between bytes and repeat: ' . $ratio . "\n";

        echo 'minimum times to repeat: ' . $used / $ratio . "\n";
    }
}

It produced the following output for me:

error handling used: 24920
repeat times 10240 reserves: 12288
ratio between bytes and repeat: 1.2
minimum times to repeat: 20766.666666667

spawnia and others added 3 commits June 3, 2022 10:49
Follow up to #42630

The error handler should be able to report exceptions
arising from exceeding the PHP memory limit. Because
of changes made to error handling, the previously
configured value is no longer sufficiently large.

A similar change was also done in Symfony a while ago,
see symfony/symfony#44327.

I used the following artisan command to determine the amount
of memory required for error handling, using a reporter that
simply writes to a file:

```php
<?php declare(strict_types=1);

namespace App\Console\Commands;

use Illuminate\Console\Command;

final class MeasureHandlerMemory extends Command
{
    protected $signature = 'measure-handler-memory';

    private int $peak;

    public function handle(): void
    {
        $this->peak = memory_get_peak_usage();

        trigger_error('', E_USER_ERROR);
    }

    public function __destruct()
    {
        $used = memory_get_peak_usage() - $this->peak;
        echo "error handling used: " . $used . "\n";

        $before = memory_get_usage();
        $times = 10240;
        $reserve = str_repeat('x', $times);
        $after = memory_get_usage() - $before;
        echo 'repeat times ' . $times . ' reserves: ' . $after . "\n";

        $ratio = $after / $times;
        echo 'ratio between bytes and repeat: ' . $ratio . "\n";

        echo 'minimum times to repeat: ' . $used / $ratio . "\n";
    }
}
```
While validating the effectiveness of #42630
in our application, I found that the call `$error = error_get_last()`
causes a tiny bit of memory usage. Thus, I think it is better
to clear memory as soon as entering the handler.
@taylorotwell taylorotwell merged commit 6d2d78c into laravel:8.x Jun 3, 2022
chu121su12 pushed a commit to chu121su12/framework that referenced this pull request Jun 6, 2022
* Increase reserved memory for error handling

Follow up to laravel#42630

The error handler should be able to report exceptions
arising from exceeding the PHP memory limit. Because
of changes made to error handling, the previously
configured value is no longer sufficiently large.

A similar change was also done in Symfony a while ago,
see symfony/symfony#44327.

I used the following artisan command to determine the amount
of memory required for error handling, using a reporter that
simply writes to a file:

```php
<?php declare(strict_types=1);

namespace App\Console\Commands;

use Illuminate\Console\Command;

final class MeasureHandlerMemory extends Command
{
    protected $signature = 'measure-handler-memory';

    private int $peak;

    public function handle(): void
    {
        $this->peak = memory_get_peak_usage();

        trigger_error('', E_USER_ERROR);
    }

    public function __destruct()
    {
        $used = memory_get_peak_usage() - $this->peak;
        echo "error handling used: " . $used . "\n";

        $before = memory_get_usage();
        $times = 10240;
        $reserve = str_repeat('x', $times);
        $after = memory_get_usage() - $before;
        echo 'repeat times ' . $times . ' reserves: ' . $after . "\n";

        $ratio = $after / $times;
        echo 'ratio between bytes and repeat: ' . $ratio . "\n";

        echo 'minimum times to repeat: ' . $used / $ratio . "\n";
    }
}
```

* Free memory in HandleExceptions::handleShutdown()

While validating the effectiveness of laravel#42630
in our application, I found that the call `$error = error_get_last()`
causes a tiny bit of memory usage. Thus, I think it is better
to clear memory as soon as entering the handler.

* Update HandleExceptions.php

Co-authored-by: Taylor Otwell <[email protected]>
@spawnia spawnia deleted the increase-reserved-memory branch June 9, 2022 09:33
QWp6t added a commit to roots/acorn that referenced this pull request Jun 26, 2022
QWp6t added a commit to roots/acorn that referenced this pull request Jun 26, 2022
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