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

Add cache of configuration files list #1628

Merged
merged 1 commit into from
Aug 31, 2016

Conversation

otakarmare
Copy link
Contributor

Performance result:

command before after
setup:uninstall 3s 0.9 s
setup:install 32s 16s

@danslo
Copy link
Contributor

danslo commented Aug 8, 2015

+1 on this. Natively on linux (on an SSD) there is no problem, but when installing via a vagrant box that gets files over a share, there's a lot of this:

And that repeats over and over.

@vpelipenko vpelipenko added the PS label Aug 10, 2015
@robbieaverill
Copy link

Looks like a great improvement to me. +1

@vancoz
Copy link

vancoz commented Aug 11, 2015

Hi @otakarmare, this approach has no cache invalidation, and Reader is questionable place to implement this caching, also it looks inconstant with other methods - getComposerJsonFiles, getActionFiles

@vancoz
Copy link

vancoz commented Aug 11, 2015

Hi @otakarmare, cache invalidation should not be an issue, have a question about getComposerJsonFiles, getActionFiles methods, probably is it good idea to add caching for them too?

@danslo
Copy link
Contributor

danslo commented Aug 11, 2015

Indeed cache invalidation doesn't seem like an issue because this cache is per-request.

@otakarmare
Copy link
Contributor Author

@vancoz I agree that getComposerJsonFiles, getActionFiles copy/paste part of logic with getConfigurationFiles and Magento/Framework/Module/Dir/Reader class has too many responsibilities, but it's another issue and I do not plan to do it in this pull request

@magento-cicd2
Copy link
Contributor

We have automated a Magento Contributor License Agreement verifier for contributions sent to our GitHub projects.
Please see the CLA agreement in the Pull Request comments.

@otakarmare
Copy link
Contributor Author

I sign CLA
any updates from magento team?

@vrann
Copy link
Contributor

vrann commented Oct 7, 2015

Can you please fix Travis builds?

@Vinai
Copy link
Contributor

Vinai commented Nov 3, 2015

@otakarmare Can you please rebase onto the current develop branch and sign the licence cla agreement? Thanks!

@mazhalai
Copy link
Contributor

@otakarmare can you please merge the latest develop and restart builds?

@mazhalai mazhalai added the Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development label Feb 23, 2016
@mazhalai
Copy link
Contributor

Internal ticket MAGETWO-50778

@vasiliyseleznev vasiliyseleznev removed the PS label Aug 3, 2016
@vkorotun vkorotun added Performance and removed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development labels Aug 3, 2016
@vkorotun vkorotun added Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development and removed linked labels Aug 22, 2016
@mmansoor-magento mmansoor-magento merged commit be22b37 into magento:develop Aug 31, 2016
mmansoor-magento pushed a commit that referenced this pull request Aug 31, 2016
 - MAGETWO-50778: [Github][PR] Add cache of configuration files list #1628
 - Merge commit 'refs/pull/1628/head' of github.com:magento/magento2 into MAGETWO-50778

# Conflicts:
#	lib/internal/Magento/Framework/Module/Dir/Reader.php
mmansoor-magento pushed a commit that referenced this pull request Aug 31, 2016
 - MAGETWO-50778: [Github][PR] Add cache of configuration files list #1628
 - refactoring
 - extending solution and applying to get getComposerJsonFiles
mmansoor-magento pushed a commit that referenced this pull request Aug 31, 2016
 - MAGETWO-50778: [Github][PR] Add cache of configuration files list #1628
 - merge mainline
okorshenko pushed a commit that referenced this pull request Oct 27, 2017
Public Pull Requests

#11761 [BACKPORT 2.1] [TASK] Incorrect minimum memory_limit references have … by @lewisvoncken
#11590 [Backport 2.1-develop] #11586 Fix duplicated crontab 2>&1 expression by @adrian-martinez-interactiv4

Fixed Public Issues

#11322 User.ini files specify 768M - Docs recommend at least 1G
#11586 Cron install / remove via command messes up stderr 2>&1 entries
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Progress: accept
Projects
None yet
Development

Successfully merging this pull request may close these issues.