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

[HttpKernel] Reduce memory consumption compiling container#18061

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

Closed

Conversation

@peteward
Copy link

QA
Branch2.3
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed ticketsfixes#18007,closes#18048
LicenseMIT
Doc PR-

Further to#18048 by@nicolas-grekas, keep the same memory savings at Container compilation time but through a str_replace of the whole container string instead of optional addition of extra opening comment* as originally proposed.

RemovestripComments and associated test.

nicolas-grekasand others added2 commitsMarch 8, 2016 13:19
… to minimise for opcache in place of stripComments, which has been removed along with tests.
@petewardpeteward changed the titleLess mem compiling containerLes mem compiling containerMar 8, 2016
@petewardpeteward changed the titleLes mem compiling containerReduce memory consumption compiling containerMar 8, 2016
@petewardpeteward changed the titleReduce memory consumption compiling container[HttpKernel] Reduce memory consumption compiling containerMar 8, 2016
$this->targetDirRegex =null;

return$code;
// Replace '/**' comment openings with '/*' for non-debug container to optimise for opcache
Copy link
Member

Choose a reason for hiding this comment

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

optimise ->optimize (we mostly use American English in code and docs) Thanks!

@nicolas-grekas
Copy link
Member

I was about to comment that this doesn't work for the general case: the str_replace cas alter strings in parameters e.g.
and stripComments can't be removed because it's used elsewhere

@peteward
Copy link
Author

Good point.

I couldn't find any reference to stripComments but I guess it would be BC break as it's public...

I'll have another go!

@peteward
Copy link
Author

Hmm unfortunately about 50% of my container is Proxy classes which are generated throughProxyGenerator and it's these classes which are being dumped with 'full' comments.

What about a more targeted preg_replace looking for opening and closing comments followed by keywords likeprivate,public,protected,class,interface ?

@sstok
Copy link
Contributor

What about a more targeted preg_replace looking for opening and closing comments followed by keywords like private, public, protected, class, interface ?

That will fail when a parameter contains actual/**, and you can't say that will never happen (the first version of the php-cs-fixer used regexes and those were hard to get right all the time). Using a regex to strip comments in a language like PHP will fail one day.

One alternative solution would be to use a different tokenizer that doesn't load the tokens in memory but rather uses a stream or something.

@peteward
Copy link
Author

indeed, quite a few examples of people struggling withtoken_get_all in these extreme cases and looking for streaming alternatives without success.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@peteward@nicolas-grekas@sstok@javiereguiluz@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp