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

Fix slow call to scandir() when uploading files #422

Merged
merged 4 commits into from
Jul 8, 2020

Conversation

joehoyle
Copy link
Member

@joehoyle joehoyle commented Jul 3, 2020

Please for give me for what I have done.

Fixes #373.

Please for give me for what I have done.

Fixes #373.
Copy link

@hm-linter hm-linter bot left a comment

Choose a reason for hiding this comment

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

Linting failed (13 errors).

(12 notices occurred in your codebase, but were on files/lines not included in this PR.)

@joehoyle joehoyle requested a review from rmccue July 3, 2020 13:27
@joehoyle
Copy link
Member Author

joehoyle commented Jul 7, 2020

@rmccue could I get a 👍 / 👎 today on this, as this is currently blocking some production errors.

Copy link
Member

@rmccue rmccue left a comment

Choose a reason for hiding this comment

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

Given this is temporary, I think it's OK, but we need to make sure we're re-evaluating this.

The linter is failing on the backtrace line though; can you add an ignore or fix the issue?

@joehoyle joehoyle requested a review from rmccue July 7, 2020 13:01
@joehoyle
Copy link
Member Author

joehoyle commented Jul 7, 2020

@rmccue I added support for the filter too here.

//
// Anyone reading this far, brace yourselves for a mighty horrible hack.
$backtrace = debug_backtrace( 0, 3 ); // phpcs:ignore PHPCompatibility.FunctionUse.ArgumentFunctionsReportCurrentValue.NeedsInspection
if ( isset( $backtrace[1]['function'] ) && $backtrace[1]['function'] === 'scandir' && isset( $backtrace[2]['function'] ) && $backtrace[2]['function'] === 'wp_unique_filename' ) {
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to bail if it hit the filter callback above, or is that already handled?

Copy link
Member Author

Choose a reason for hiding this comment

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

This won't be triggered, as the backtrace doesn't reflect our own scandir call.

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha.

@joehoyle joehoyle merged commit a025465 into namespaces Jul 8, 2020
@joehoyle joehoyle deleted the fix-slow-scandir branch July 8, 2020 09:07
joehoyle added a commit that referenced this pull request Jul 8, 2020
joehoyle added a commit that referenced this pull request Jul 8, 2020
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