Skip to content

Use symfony-clean-tags plugin to improve performance and resolve install/update errors #452

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Use symfony-clean-tags plugin to improve performance and resolve install/update errors #452

wants to merge 1 commit into from

Conversation

lcatlett
Copy link

@lcatlett lcatlett commented Jan 28, 2019 •
edited
Loading

Fixes #439

Having zaporylie/composer-drupal-optimizations in the root composer.json appears to cause issues with autoloading classes in the correct place when using the installers and composer-merge plugins and also hard codes the symfony version constraint. Hard coding this constraint may cause issues when updating core and other dependent packages unless zaporylie/composer-drupal-optimizations is actively maintained. Swapping out composer-drupal-optimizations for the symfony-clean-tags-composer-plugin will allow for the same performance improvements without the additional maintenance and update issues. Benchmarks (below) show that this plugin might even perform slightly better.

Changes proposed:

Use rubenrua/symfony-clean-tags-composer-plugin rather than zaporylie/composer-drupal-optimizations

Steps to reproduce:

create a new drupal-project and atempt to update core or another package dependent on core. Observe the "Could not scan for classes" errors:

~/sites/some-dir
❯ composer update drupal/core
Could not scan for classes inside "docroot/core/lib/Drupal.php" which does not appear to be a file nor a folder

Benchmarks:

with zaporylie/composer-drupal-optimizations [240.5MB/6.11s]

~/sites/some-dir 32s
❯ composer update nothing --profile -v
[11.6MB/0.31s] Loading composer repositories with package information
[240.5MB/6.11s] Memory usage: 240.51MB (peak: 307.38MB), time: 6.11s

drupal-project without optimization plugins [321.1MB/16.81s]

~/sites/some-dir 12s
❯ composer remove zaporylie/composer-drupal-optimizations
Package operations: 0 installs, 0 updates, 1 removal
  - Removing zaporylie/composer-drupal-optimizations (1.0.2)
Writing lock file
Generating autoload files

~/sites/some-dir 8s
❯ composer update nothing --profile -v
[11.4MB/0.07s] Loading composer repositories with package information
[321.1MB/16.81s] Memory usage: 321.11MB (peak: 1115.54MB), time: 16.81s

With rubenrua/symfony-clean-tags-composer-plugin [220.1MB/2.82s]

~/sites/some-dir 18s
❯ composer require rubenrua/symfony-clean-tags-composer-plugin

~/sites/some-dir 21s
❯ COMPOSER_MEMORY_LIMIT=-1 composer config extra.symfony.require 3.4.*

~/sites/some-dir
❯ composer update nothing --profile -v
[9.7MB/0.06s] > pre-update-cmd: DrupalProject\composer\ScriptHandler::checkComposerVersion
[11.5MB/0.06s] Loading composer repositories with package information
[154.5MB/1.13s] Restricting packages listed in "symfony/symfony" to "3.4.0"
[243.7MB/2.60s] Dependency resolution completed in 0.022 seconds
[244.1MB/2.61s] Analyzed 7418 packages to resolve dependencies
[244.1MB/2.61s] Analyzed 36401 rules to resolve dependencies
[220.1MB/2.82s] Memory usage: 220.08MB (peak: 245.9MB), time: 2.82s

@lcatlett
Remove composer-drupal-optimizations package in favor of symfony-clea…
e500d70 
…n-tags-composer-plugin to resolve autoloading errors.
@weitzman
Copy link
Contributor

hard codes the symfony version constraint

That does look bad. Has anyone else used symfony-clean-tags?

@normanlolx
Copy link
Collaborator

normanlolx commented Jan 29, 2019 •
edited
Loading

Linking zaporylie/composer-drupal-optimizations.
Linking rubenrua/symfony-clean-tags-composer-plugin.

@webflo
Copy link
Member

webflo commented Jan 29, 2019

I haven't used rubenrua/symfony-clean-tags-composer-plugin yet. Wait and see what @zaporylie has to say.

But if we use symfony-clean-tags-composer-plugin we should change the description in the README. When updating the core you might have to change the configuration (extra.symfony.require).

I would prefer a preconfigured package that we can add to the composer update common. Nothing should break, if you keep using the old version of "composer-drupal-optimizations". It is just not as performant as the newer version would be. Because the lower version constraint is not updated yet.

@zaporylie
Copy link
Contributor

Sorry for a delay, been busy yesterday. And thank you @webflo for pinging me - I wasn't aware of this discussion (nor acquia/blt#3336 for that matter). I didn't have much time to look in depth into all mentioned issues but let me, at the very least, provide some background/feedback:

  • the zaporylie/composer-drupal-optimizations was created as a POC to Removing legacy tags significantly reduces memory and CPU usage symfony/flex#378 to see if Drupal can benefit from the same approach as symfony/flex
  • initially, symfony legacy constraint was limited to 3.4 to match Drupal 8.5 requirements. In addition, drupal/core:^8.5 was added as a requirement in composer.json in order to prevent the module from being installed on older versions of drupal/core
  • the first round of feedback, triggered by https://twitter.com/zaporylie/status/1018470659117154304 resulted in the proposal of making the legacy version constraint configurable. I have proposed Allow setting custom lowest-tags in root composer.json zaporylie/composer-drupal-optimizations#5 and pinged those who have requested it, but got no reply whatsoever so PR got stale - I just didn't think people care that much about the feature
  • that leads to the strategic decisions I've made - I decided to bundle (what you call hard coding) lowest symfony tags directly into the plugin to provide zero-config experience. In the PR I've mentioned above, you can see I decided on hybrid approach - make the legacy tags configurable but, at the same time, provide sensible defaults so most of the people can gain from the zero-config approach
  • zaporylie/composer-drupal-optimizations was made as a placeholder for all kinds of composer performance improvements for Drupal, not just the symfony legacy tags. ex. Consider a fork blacklist zaporylie/composer-drupal-optimizations#4
  • according to packagist, the rubenrua/symfony-clean-tags-composer-plugin was downloaded over 6k times, while zaporylie/composer-drupal-optimizations over 30k times - so the latter was tested by 5 times larger user base than former
  • TBH I wasn't aware zaporylie/composer-drupal-optimizations has finally landed in drupal-composer/drupal-project - I guess that happened only 4 days ago so I could have missed it. I am happy that even more people will test it now and, hopefully, provide some feedback to the package I maintain. That being said - I am committed to maintain the package in question. I am also open for additional maintainers
  • the difference in the performance is, most likely, due to dev-master removed in rubenrua/symfony-clean-tags-composer-plugin but kept intact in zaporylie/composer-drupal-optimizations
  • I've been able to reproduce some of the issues that have been reported to me and working to resolve them ASAP

@zaporylie zaporylie mentioned this pull request Jan 30, 2019
Closed
@lcatlett lcatlett mentioned this pull request Feb 2, 2019
Closed
@zaporylie
Copy link
Contributor

New release to zaporylie/composer-drupal-optimizations was tagged earlier today. It allows you to customize required symfony tags in root level composer.json AND also provides default based on drupal/core version constraint - the hybrid approach I mentioned above.

Can this issue be closed now?

@lcatlett
Copy link
Author

thank you @zaporylie !!

@lcatlett lcatlett closed this Feb 25, 2019
@lcatlett lcatlett mentioned this pull request Mar 19, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

composer installation "cannot find specified file"?
5 participants
@lcatlett @weitzman @normanlolx @webflo @zaporylie