Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Debug][ErrorHandler] Increased the reserved memory from 10k to 32k#44327

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:4.4fromsakalys:oom_handler_increase_reserved_memory__ticket_40824
Nov 29, 2021
Merged

[Debug][ErrorHandler] Increased the reserved memory from 10k to 32k#44327

fabpot merged 1 commit intosymfony:4.4fromsakalys:oom_handler_increase_reserved_memory__ticket_40824
Nov 29, 2021

Conversation

@sakalys
Copy link
Contributor

@sakalyssakalys commentedNov 29, 2021
edited by nicolas-grekas
Loading

QA
Branch?4.4
Bug fix?yes
New feature?no
Deprecations?no
TicketsFix#40824
LicenseMIT
Doc PRn/a

The ErrorHandler's job includes handling out of memory (OOM) exceptions,
therefore it is important that that feature works. In the current state,
when handling OOM exceptions, the error handler produces an OOM error
itself, because the old 10kB reserve is apparently not enough anymore.
To mitigate that, the reserved memory gets bumped to 32k (which is enough).

Note I'm not familiar with the whole open source submitting process, so any feedback and instructions on how to improve this PR are welcome.
I am not sure on how to write a unit test to test something like that (talking about the issue here#40824).

The ErrorHandler's job includes handling out of memory (OOM) exceptions,therefore it is imoprtant that that feature works. In the current state,when handling OOM exceptions, the error handler produces an OOM erroritself, because the old 10k reserve is apparently not enough anymore.To mitigate that, the reserved memory gets bumped to 32k (which is enouogh).
@carsonbot
Copy link

Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has acontribution guide which I suggest you to read.

In short:

  • Always add tests
  • Keep backward compatibility (seehttps://symfony.com/bc).
  • Bug fixes must be submitted against the lowest maintained branch where they apply (seehttps://symfony.com/releases)
  • Features and deprecations must be submitted against the 5.4 branch.

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor!
If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

@carsonbotcarsonbot changed the title[Debug] [ErrorHandler] Increased the reserved memory from 10k to 32k[Debug][ErrorHandler] Increased the reserved memory from 10k to 32kNov 29, 2021
@fabpot
Copy link
Member

Thank you@sakalys.

sakalys reacted with thumbs up emoji

@fabpotfabpot merged commitcacce1c intosymfony:4.4Nov 29, 2021
@sakalyssakalys deleted the oom_handler_increase_reserved_memory__ticket_40824 branchNovember 29, 2021 10:36
This was referencedNov 29, 2021
This was referencedDec 29, 2021
taylorotwell added a commit to laravel/framework that referenced this pull requestJun 3, 2022
* Increase reserved memory for error handlingFollow up to#42630The error handler should be able to report exceptionsarising from exceeding the PHP memory limit. Becauseof changes made to error handling, the previouslyconfigured value is no longer sufficiently large.A similar change was also done in Symfony a while ago,seesymfony/symfony#44327.I used the following artisan command to determine the amountof memory required for error handling, using a reporter thatsimply 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#42630in our application, I found that the call `$error = error_get_last()`causes a tiny bit of memory usage. Thus, I think it is betterto clear memory as soon as entering the handler.* Update HandleExceptions.phpCo-authored-by: Taylor Otwell <taylor@laravel.com>
chu121su12 pushed a commit to chu121su12/framework that referenced this pull requestJun 6, 2022
* Increase reserved memory for error handlingFollow up tolaravel#42630The error handler should be able to report exceptionsarising from exceeding the PHP memory limit. Becauseof changes made to error handling, the previouslyconfigured value is no longer sufficiently large.A similar change was also done in Symfony a while ago,seesymfony/symfony#44327.I used the following artisan command to determine the amountof memory required for error handling, using a reporter thatsimply 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 oflaravel#42630in our application, I found that the call `$error = error_get_last()`causes a tiny bit of memory usage. Thus, I think it is betterto clear memory as soon as entering the handler.* Update HandleExceptions.phpCo-authored-by: Taylor Otwell <taylor@laravel.com>
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@ycerutoycerutoAwaiting requested review from ycerutoyceruto is a code owner

Assignees

No one assigned

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

4 participants

@sakalys@carsonbot@fabpot@nicolas-grekas

[8]ページ先頭

©2009-2025 Movatter.jp