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

Asset installation of 2.0.13 very slow with cap installed #149

Closed
schmunk42 opened this issue Nov 3, 2017 · 25 comments
Closed

Asset installation of 2.0.13 very slow with cap installed #149

schmunk42 opened this issue Nov 3, 2017 · 25 comments
Assignees
Milestone

Comments

@schmunk42
Copy link
Contributor

What steps will reproduce the problem?

Install the basic-app with global fxp/cap installation

What's expected?

  • super-fast installation (with filled caches)

What do you get instead?

  • slow installation, due to a lot of API requests

Additional info

Q A
Yii vesion any
PHP version any
Operating system any

Introduced by 395e9c7

Whether you like the asset-plugin or not is no question here, but removing all the stuff introduced without BC-break in 120d355 - one day earlier, just makes no sense to me.

If you have a global cap installation you can not use this anymore, please do not force this onto the developers, I have plenty of reasons not to use AP in many projects.
The settings and info have no effect, if do not have fxp/cap installed, they simply take care that things work as before, just almost as fast as with AP.

Such a removal has to be done in 2.1 not earlier. I'd assume the majority of users still have the plugin installed, since it was the recommended way for years.

If you want to disable fxp/cap, you could add a config option to composer.json for that, but I think this should also be the developers choice.

@samdark
Copy link
Member

samdark commented Nov 3, 2017

That would only affect new projects, not existing ones since that's a template.

@schmunk42
Copy link
Contributor Author

Sorry, I don't get it, what do you mean with "that"? I am just saying that there was a perfect configuration for all use-cases, now an essential speed upgrade is removed for no reason.

@nkovacs
Copy link
Contributor

nkovacs commented Nov 8, 2017

This is why I asked for an option to disable c-a-p from composer.json: fxpio/composer-asset-plugin#249
You should do that.

@schmunk42
Copy link
Contributor Author

Depends on what you want, with the right config the performance is almost the same.
It should be up to the developer to choose.

@nkovacs
Copy link
Contributor

nkovacs commented Nov 8, 2017

But the current configuration is bad regardless. This template currently assumes that you do not have c-a-p installed globally, but that assumption is wrong. It should therefore disable c-a-p explicitly, otherwise c-a-p will run, causing slowness and problems.

@schmunk42
Copy link
Contributor Author

But the current configuration is bad regardless. This template currently assumes that you do not have c-a-p installed globally, but that assumption is wrong.

It makes no assumption about that, it just does not disable the plugin by default, which is fine.

It should therefore disable c-a-p explicitly, otherwise c-a-p will run, causing slowness and problems.

Not anymore with the configuration which was removed.

If you want to disable c-a-p, use the ENV var or a composer.json setting. It's the default behavior, that an installed plugins runs.

@schmunk42
Copy link
Contributor Author

Should be...

"config":{
  "fxp-asset":{		
    "enabled": true
  }
}

or false 😄

@nkovacs
Copy link
Contributor

nkovacs commented Nov 8, 2017

Even with the git configuration, it is slower than not running the plugin at all. Plus with c-a-p enabled, you'll get duplicated packages: both c-a-p and asset-packagist will give composer's dependency solver two separate packages, with all their versions, which will make it slower and cause it to use more memory.
I've had this happen before when packages were duplicated by satis. If the exact same version was available from both, composer randomly installed one or the other on each composer update. I don't know if this bug has since been fixed, but probably not: composer/composer#2959 (comment)

@nkovacs
Copy link
Contributor

nkovacs commented Nov 9, 2017

with asset-packagist, c-a-p disabled:

Memory usage: 173.83MB (peak: 251.52MB), time: 2.58s
Elapsed (wall clock) time (h:mm:ss or m:ss): 0:02.67
Maximum resident set size (kbytes): 281276

with asset-packagist, c-a-p enabled and set to use git, last 2 days (like the previous config), first run:

Memory usage: 176.21MB (peak: 255.26MB), time: 123.08s
Elapsed (wall clock) time (h:mm:ss or m:ss): 2:03.17
Maximum resident set size (kbytes): 285460

second run:

Memory usage: 176.22MB (peak: 255.44MB), time: 72.39s
Elapsed (wall clock) time (h:mm:ss or m:ss): 1:12.58
Maximum resident set size (kbytes): 285444

More than a minute vs 2.5 seconds.

without using git, i.e. if you have c-a-p installed and you try to use this template:

Memory usage: 176.28MB (peak: 255.33MB), time: 99.27s
Elapsed (wall clock) time (h:mm:ss or m:ss): 1:39.36
Maximum resident set size (kbytes): 285656

Wait, wasn't git supposed to be faster?

In all cases I ran composer update --profile without deleting vendor, and there were no updates. Feel free to benchmark other cases, but I think this is convincing enough.

So clearly the plugin should be disabled by default. If someone wants to use c-a-p, they can remove asset-packagist and re-enable the plugin in their composer.json.

@samdark
Copy link
Member

samdark commented Nov 9, 2017

Considering @nkovacs benchmark, I think that removal of CAP from the template is a good move and should stay like that.

@samdark samdark closed this as completed Nov 9, 2017
@nkovacs
Copy link
Contributor

nkovacs commented Nov 9, 2017

But you need to disable it in composer.json. It is currently enabled if you have it installed globally, which you probably have if you've used this template before.

@schmunk42
Copy link
Contributor Author

schmunk42 commented Nov 9, 2017

@nkovacs With which version of the asset-plugin did you run your benchmarks? The bower URL was updated in 1.4.2 - if you are below that you are loosing a lot of time.

My benchmarks for the yii2-basic-app are (2nd run):

FXP_ASSET__ENABLED=0 composer update --profile
[157.9MB/2.32s] Memory usage: 157.87MB (peak: 213.47MB), time: 2.32s

FXP_ASSET__ENABLED=1 composer update --profile
[159.9MB/3.98s] Memory usage: 159.87MB (peak: 229.58MB), time: 3.98s

@nkovacs
Copy link
Contributor

nkovacs commented Nov 9, 2017

You're right, I was using 1.3.
New benchmarks, this time without vendor and composer.lock (although it doesn't seem to make much of a difference, except with git):
c-a-p disabled: Memory usage: 172.96MB (peak: 249.65MB), time: 4.01s
c-a-p enabled: Memory usage: 176.29MB (peak: 256.99MB), time: 43.37s
c-a-p enabled, git: Memory usage: 176.39MB (peak: 257.09MB), time: 7.42s

All the packages were already downloaded and cached.
So the current configuration of the app (without git) is still significantly slower, plus I still believe it is not a good idea to mix asset-packagist and c-a-p.
But if you want to use c-a-p with git, that's fine too. In that case, you should remove asset-packagist.

@schmunk42
Copy link
Contributor Author

Having both in the composer.json does not does not really hurt, from what I know this is the only issue so far, when using both: hiqdev/asset-packagist#72

The thing is, if you have c-a-p installed, you want to have the fast config. If you don't have c-a-p (can be removed from the docs, I don't mind) a basic installation works fine (and even slightly faster) with AP.

@nkovacs
Copy link
Contributor

nkovacs commented Nov 9, 2017

But subtle bugs like that are exactly why it's not a good idea to have both c-a-p and asset-packagist active in a project. Plus you'll get inconsistent behavior if you're working with someone else who doesn't have c-a-p installed.

@schmunk42
Copy link
Contributor Author

That's true but only to a certain degree, since AP will not be used, if you have c-a-p installed.
AP uses c-a-p under the hood, so it should not be that inconsistent.

But they are valid points, I think we can agree on:

  • if you want to disable c-a-p, use enabled=false in composer.json
  • if you want to disable AP, remove it from the repositories section

I'd be fine with enabled=false in the app-templates, but along with the "correct" configuration for the other options.

@nkovacs
Copy link
Contributor

nkovacs commented Nov 9, 2017

That's true but only to a certain degree, since AP will not be used, if you have c-a-p installed.

But it will. c-a-p will feed composer the bower/npm packages it finds, but composer will also get those packages from asset-packagist, and both sets of packages will end up in composer's dependency solver. That's why you get bugs like hiqdev/asset-packagist#72

@schmunk42
Copy link
Contributor Author

Makes perfect sense, although when using both it looks like c-a-p always takes precedence - "AP will not be used" is the wrong term here.

Both

Dependency resolution completed in 2.549 seconds
Analyzed 22527 packages to resolve dependencies
Analyzed 608753 rules to resolve dependencies

AP only

Dependency resolution completed in 2.175 seconds
Analyzed 21624 packages to resolve dependencies
Analyzed 604495 rules to resolve dependencies

c-a-p only

Dependency resolution completed in 1.139 seconds
Analyzed 16625 packages to resolve dependencies
Analyzed 299342 rules to resolve dependencies

Tested with phd5

Look how much faster c-a-p is 😸 - jokes aside - there should only be one active one.

@schmunk42
Copy link
Contributor Author

@SilverFire @samdark the plugin is disabled, but it should still have a useable config

@schmunk42 schmunk42 reopened this Nov 12, 2017
@schmunk42
Copy link
Contributor Author

FYI: hiqdev/asset-packagist#74

@samdark
Copy link
Member

samdark commented Feb 6, 2018

@SilverFire would you please take a look at this one and related ones? Thanks.

@schmunk42
Copy link
Contributor Author

We need at least something in the docs how to configure fxpio/cap.

@samdark
Copy link
Member

samdark commented Feb 17, 2018

I don't think we should encourage using it.

@samdark samdark closed this as completed Feb 17, 2018
samdark pushed a commit to yiisoft/yii2-app-advanced that referenced this issue Feb 17, 2018
@schmunk42
Copy link
Contributor Author

I give up ... if you think there should be no note about the correct configuration of a piece of software which was required for years and which is still required if you use just one asset bundle which has sadly hundreds of releases.

I must state that the way how the whole (silent) replacement of fxpio/cap was handled is far from ideal. People were running into numerous issues with renamed vendor/bower-asset folders, which is still in the core due to BC, but changed in the yiisoft application templates.

Since the information about fxpio/cap has vanished you need to know that you either sill need to install it for older applications or add the AP-repo to the composer.json file, but I guess you see that as part of the application docs.

Just my 2 cents.

@samdark
Copy link
Member

samdark commented Feb 21, 2018

http://www.yiiframework.com/doc-2.0/guide-structure-assets.html#bower-npm-assets

yangke687 pushed a commit to yangke687/yii2-eCommerce-mvp that referenced this issue Jan 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants