apache / cordova-cli

Apache Cordova CLI
Apache License 2.0
930 stars 338 forks source link

Improve startup speed/performance of CLI #406

Open janpio opened 5 years ago

janpio commented 5 years ago

Even with telemetry turned off, our CLI seems a bit slow sometimes:

C:\Users\Jan>cordova -v | gnomon
   1.1532s   8.1.2 (cordova-lib@8.1.1)
   0.0014s

     Total   1.1572s

C:\Users\Jan>node -v | gnomon
   0.0116s   v10.15.1
   0.0010s

     Total   0.0135s

C:\Users\Jan>npm -v | gnomon
   0.6859s   6.4.1
   0.0016s

     Total   0.6893s

C:\Users\Jan>ngrok -v | gnomon
   0.0131s   ngrok version 2.2.8
   0.0017s

     Total   0.0157s

(Yes, I am working on a not very powerful machine here) (ionic is even slower with >7 seconds - but that's no excuse ;) )

erisu commented 5 years ago

@janpio Can we get an update with Cordova CLI 9 on the same computer.

Running cordova -v | gnomon five times on my macOS computer, has an average of 0.47674s

janpio commented 5 years ago

No improvement:

E:\Projects           
λ cordova -v | gnomon 
   1.5790s   9.0.0    
   0.0019s            

     Total   1.5849s  

E:\Projects           
λ cordova -v | gnomon 
   1.6072s   9.0.0    
   0.0019s            

     Total   1.6105s  

E:\Projects           
λ cordova -v | gnomon 
   1.5453s   9.0.0    
   0.0026s            

     Total   1.5522s  

E:\Projects           
λ cordova -v | gnomon 
   1.5194s   9.0.0    
   0.0019s            

     Total   1.5253s  

E:\Projects           
λ node -v | gnomon    
   0.0117s   v10.15.1 
   0.0016s            

     Total   0.0159s  

(Windows 10)

dpogue commented 5 years ago

It seems like telemetry negatively affects this:

$ cordova -v | gnomon
   1.3160s   9.0.0
   0.0005s   

     Total   1.3167s
$ CI=1 cordova -v | gnomon
   0.4831s   9.0.0
   0.0005s   

     Total   0.4842s
janpio commented 5 years ago

My timings were with telemetry already disabled, with it on it adds ~0.2 to ~0.5 seconds.

The relevant bit for me here was how cordova -v is slower than other Node based CLIs like npm or ngrok.

raphinesse commented 5 years ago

I did some profiling and about half of the time is spent by require('insight'). When you drill down a bit, you can see that the actual culprit that causes the long loading time is Rx.js which is required by inquirer, a dependency of insight.

Roughly the other half is spent on loading cordova-lib. There's a few things I wanted to do about that for some time, but I haven't gotten to it. The Problem with improving the loading time there is that a lot of modules require way more than they would have to. I have some WIP to improve that, but it's quite some work since test doubles are affected when changing requires etc.

My times for cordova -v are as follows:

raphinesse commented 5 years ago

Follow-up: inquirer already fixed the problem by only importing what they use from rxjs. But insight does still use an older version of inquirer. Edit: tested with the latest version of inquirer but it only improves the loading time by around 100ms. They are working on a major refactor that should improve it even more though.

For cordova-lib cordova.serve was the biggest offender regarding module load time. However, this is to be taken with a grain of salt because of Node's module caching. Meaning if you don't load cordova.serve, other modules that share the same dependencies could take longer to load, since their dependencies haven't been cached by loading cordova.serve.

raphinesse commented 5 years ago

Well the question in the title is answered. Should we close this or rename it to something more actionable?

janpio commented 5 years ago

Renamed to a task that benefits of both the measurements and your analysis.

dpogue commented 5 years ago

See also the discussion from a while back at https://github.com/apache/cordova-discuss/pull/70 (and subsequent PR to remove lazy-loading from cordova-lib: https://github.com/apache/cordova-lib/pull/562)

raphinesse commented 5 years ago

Thanks for the links @dpogue! I think the reason while lazy loading did not bring any real benefits as it was being dropped is that you basically load almost everything of lib, once you load something. That can be improved. I have some work in progress regarding that lying around. Don't know when I'll get to continue it though.

dpogue commented 1 week ago

Telemetry code has been removed, so hopefully that improves the startup speed a bit