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

[Finder] Use a lazyIterator to close files descriptors when no longer used#40040

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.4fromjderusse:finder-lazy
Feb 12, 2021

Conversation

@jderusse
Copy link
Member

QA
Branch?4.4
Bug fix?yes
New feature?no
Deprecations?no
Ticketsfix#35508
LicenseMIT
Doc PR-

TheRecursiveDirectoryIterator class open the file on__construct.
Because we Inject an instance ofRecursiveDirectoryIterator inside the \AppendIterator` class, php opens a lot of file even before iterating on it.

This PR adds a newLazyIterator class that instantiate the decorated class only when something starts iterating on it.
When the iteration is over, it unset the variable to close let the decorated class clean things (ie. close the files)

Incognito reacted with heart emoji
@carsonbotcarsonbot added this to the4.4 milestoneJan 30, 2021
@jderussejderusseforce-pushed thefinder-lazy branch 2 times, most recently fromb984368 toc1b3e9bCompareJanuary 30, 2021 17:12
@jderussejderusse changed the titleUse a lazyintertor to close files descriptors when no longer used[Finder] Use a lazyIterator to close files descriptors when no longer usedJan 30, 2021
@carsonbot
Copy link

Hey!

This is.. this is amazing. Thank you!

I think@hurricane-voronin has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

@ro0NL
Copy link
Contributor

ro0NL commentedFeb 5, 2021•
edited
Loading

im wondering ifIteratorAggregate would be more simple to implement

eg.append(LazyIterator::create($callable))

class LazyIterator implements IteratorAggregate {   getIterator() {     yield from ($this->callback)();   }  static create($callback) {     return new IteratorIterator(new self($callback));  }}

roughly :)

@jderusse
Copy link
MemberAuthor

jderusse commentedFeb 5, 2021•
edited
Loading

im wondering ifIteratorAggregate would be more simple to implement

That's not possible,AppendIterator::append expect a instance ofIterator (in fact, everything in Finder is based onIterator)

edit: I missed the trick withIteratorIterator... let me give a try

@jderusse
Copy link
MemberAuthor

Refactored to use IteratorAggregate .
Thank you for the good idea@ro0NL

Copy link
Contributor

@ro0NLro0NL left a comment

Choose a reason for hiding this comment

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

👍🏻 for LazyIterator as feature also, given Finder::append() supports aggregates, eg.Finder::append(new LazyIterator($callable)) works.

@fabpot
Copy link
Member

Thank you@jderusse.

@fabpotfabpot merged commitdc20a31 intosymfony:4.4Feb 12, 2021
/**
* @author Jérémy Derussé <jeremy@derusse.com>
*/
class LazyIterator implements \IteratorAggregate
Copy link
Member

Choose a reason for hiding this comment

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

should we mark it as@internal ?

Copy link
Member

Choose a reason for hiding this comment

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

I agree, see#40163

fabpot added a commit that referenced this pull requestFeb 12, 2021
This PR was merged into the 4.4 branch.Discussion----------[Finder] mark the LazyIterator class as internal| Q             | A| ------------- | ---| Branch?       | 4.4| Bug fix?      | no| New feature?  | no| Deprecations? | no| Tickets       |#40040 (comment)| License       | MIT| Doc PR        |Commits-------4a2c996 mark the LazyIterator class as internal
This was referencedMar 4, 2021
joshtrichards pushed a commit to joshtrichards/symfony-finder that referenced this pull requestApr 26, 2024
This PR was merged into the 4.4 branch.Discussion----------[Finder] mark the LazyIterator class as internal| Q             | A| ------------- | ---| Branch?       | 4.4| Bug fix?      | no| New feature?  | no| Deprecations? | no| Tickets       |symfony/symfony#40040 (comment)| License       | MIT| Doc PR        |Commits-------8e69df8 mark the LazyIterator class as internal
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@stofstofstof requested changes

@xabbuhxabbuhxabbuh left review comments

@fabpotfabpotfabpot approved these changes

+1 more reviewer

@ro0NLro0NLro0NL approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

6 participants

@jderusse@carsonbot@ro0NL@fabpot@stof@xabbuh
[8]ページ先頭
©2009-2025 Movatter.jp