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

Glob and scandir without sorting are faster #9208

Merged
merged 2 commits into from Jun 29, 2018

Conversation

Projects
None yet
6 participants
@lfluvisotto
Contributor

lfluvisotto commented Jun 23, 2018

Questions Answers
Branch? develop
Description? See > https://www.exakat.io/php-likes-sorting/
Type? improvement
Category? CO
BC breaks? no
Deprecations? no
Fixed ticket? no
How to test? Run the store

This change is Reviewable

@prestonBot prestonBot added the develop label Jun 23, 2018

@mickaelandrieu

This comment has been minimized.

Contributor

mickaelandrieu commented Jun 25, 2018

Hello @lfluvisotto, this may be the expected behavior to sort the data... ping @Quetzacoalt91 ?

@Quetzacoalt91

This comment has been minimized.

Member

Quetzacoalt91 commented Jun 25, 2018

Indeed, I see some code related to module & core upgrades is concerned.

Note that applying the files in an incorrect order may have bad consequences on the shop, the default order follows each release version properly.

@lfluvisotto

This comment has been minimized.

Contributor

lfluvisotto commented Jun 25, 2018

@mickaelandrieu @Quetzacoalt91

I will double check the changes, most of them doesn't need to be ordered.

Use ordering scandir where doesn't need is a bottleneck/pitfall to the performance/speed.

@PierreRambaud

This comment has been minimized.

Contributor

PierreRambaud commented Jun 26, 2018

I'm agree with @lfluvisotto ordering when it's not needed for display is useless and have performance issues.

@lfluvisotto

This comment has been minimized.

Contributor

lfluvisotto commented Jun 26, 2018

@mickaelandrieu @Quetzacoalt91 @PierreRambaud

I will make some remarks in the commit indicating that is not necessary being ordered the scandir or glob, so I will do that during my free time.

@@ -867,7 +867,7 @@ public function getClasses($path = null)
}
$classes = array();
foreach (scandir($dir) as $file) {
foreach (scandir($dir, SCANDIR_SORT_NONE) as $file) {

This comment has been minimized.

@lfluvisotto

lfluvisotto Jun 26, 2018

Contributor

We have below sort($classes) so it can be with SCANDIR_SORT_NONE

@@ -622,7 +622,7 @@ private function cleanCache()
foreach ($arrayToClean as $dir) {
if (file_exists($dir)) {
foreach (scandir($dir) as $file) {
foreach (scandir($dir, SCANDIR_SORT_NONE) as $file) {

This comment has been minimized.

@lfluvisotto

lfluvisotto Jun 26, 2018

Contributor

cleanCache() so it can be with SCANDIR_SORT_NONE

@PierreRambaud PierreRambaud changed the title from CO: glob and scandir no sorting are faster to CO: glob and scandir without sorting are faster Jun 28, 2018

@marionf marionf self-assigned this Jun 28, 2018

@marionf marionf added QA ✔️ and removed waiting for QA labels Jun 28, 2018

@marionf marionf removed their assignment Jun 28, 2018

@mickaelandrieu

This comment has been minimized.

Contributor

mickaelandrieu commented Jun 29, 2018

Thank you @lfluvisotto and everyone for the reviews

@mickaelandrieu mickaelandrieu added this to the 1.7.5.0 milestone Jun 29, 2018

@mickaelandrieu mickaelandrieu merged commit 841fa65 into PrestaShop:develop Jun 29, 2018

2 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@eternoendless eternoendless changed the title from CO: glob and scandir without sorting are faster to Glob and scandir without sorting are faster Aug 27, 2018

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