Just replaced all instances in cordova-cli and plugman, and removed shelljs from node_modules to make *sure* we don't use that one anywhere. (also forced a syntax error to sanity check that I'm sure I'm using the right local version).
Results are zero noticeable improvement in perf. May still be worth the switch for the file handle issues or whatnot, but I don't predict a perf win. -Michal On Tue, Aug 6, 2013 at 10:22 AM, Michal Mocny <[email protected]> wrote: > I didn't replace it inside plugman, but assumed that for "plugin ls" and > "platform ls" it would not call into plugman.. perhaps it always does for > some setup, and I'll try replacing *all* instances for good measure. > > > On Tue, Aug 6, 2013 at 9:57 AM, Andrew Grieve <[email protected]>wrote: > >> Whenever I run "createmobilespec.sh", I get an EMFILE error, which is >> caused by shelljs.exec leaking file handles, so it must be calling it a >> good number of times. >> >> Did you replace it with shelljs-ffi in plugman as well? >> >> Somewhat related - I tried swapping in shelljs-ffi in coho, but it fails >> after a while as well due to some cryptic FILE related error. >> >> >> On Fri, Aug 2, 2013 at 5:23 PM, Michal Mocny <[email protected]> wrote: >> >> > Not nested as in, for any single command we don't launch many >> subprocesses, >> > maybe just a single invocation. So, the bulk of the time is spent >> outside >> > of the overhead of this single slow utility. I think! >> > >> > >> > On Fri, Aug 2, 2013 at 3:49 PM, Anis KADRI <[email protected]> >> wrote: >> > >> > > not nested as in we're using it in synchronous way ? >> > > >> > > On Fri, Aug 2, 2013 at 12:30 PM, Michal Mocny <[email protected]> >> > wrote: >> > > > Just did a few quick tests: while shelljs exec may be slow, seems >> we >> > are >> > > > not using it in a nested way (which seems quite obvious). >> > > > >> > > > I wrote a quick benchmark to: >> > > > >> > > > cordova plugin ls >> > > > cordova platform ls >> > > > cordova prepare >> > > > >> > > > in a loop a bunch of times and see almost zero time difference >> > replacing >> > > > shelljs with shelljs-ffi. I would imagine that means very little >> > benefit >> > > > to replacing with child_process as well, at least for a perf >> > standpoint. >> > > > It may still be worth doing for other benefits, but in the >> interest of >> > > > trying to make cordova-cli faster, I'm going to benchmark for other >> > > > bottlenecks. >> > > > >> > > > -Michal >> > > > >> > > > >> > > > On Thu, Jul 25, 2013 at 1:57 PM, Filip Maj <[email protected]> wrote: >> > > > >> > > >> cheers >> > > >> >> > > >> On 7/25/13 10:54 AM, "Andrew Grieve" <[email protected]> wrote: >> > > >> >> > > >> >Issues created. >> > > >> >https://issues.apache.org/jira/browse/CB-4398 >> > > >> >https://issues.apache.org/jira/browse/CB-4397 >> > > >> > >> > > >> > >> > > >> >On Thu, Jul 25, 2013 at 1:41 PM, Filip Maj <[email protected]> wrote: >> > > >> > >> > > >> >> Chalk it up as another issue. >> > > >> >> >> > > >> >> Feel free to help out with CLI/plugman tagged issues everyone. >> Lots >> > > of >> > > >> >> work there! ;) >> > > >> >> >> > > >> >> On 7/25/13 10:39 AM, "Anis KADRI" <[email protected]> wrote: >> > > >> >> >> > > >> >> >xD. +1 for child_process because it comes with node too eh :) >> > > >> >> > >> > > >> >> >On Thu, Jul 25, 2013 at 10:36 AM, Filip Maj <[email protected]> >> > wrote: >> > > >> >> >> Changed your named to Gmail now huh Jesse? >> > > >> >> >> >> > > >> >> >> On 7/25/13 10:32 AM, "Gmail" <[email protected]> >> wrote: >> > > >> >> >> >> > > >> >> >>>+1 to child_process or leave it slow. >> > > >> >> >>> >> > > >> >> >>>Sent from my iPhone >> > > >> >> >>> >> > > >> >> >>>On Jul 25, 2013, at 10:28 AM, Filip Maj <[email protected]> >> wrote: >> > > >> >> >>> >> > > >> >> >>>> Yeah that¹s how a lot of the "hey this works native on >> > windows" >> > > >> >> >>>>modules >> > > >> >> >>>>on >> > > >> >> >>>> npm works: they have to compile it D: >> > > >> >> >>>> >> > > >> >> >>>> On 7/25/13 10:26 AM, "Andrew Grieve" <[email protected] >> > >> > > wrote: >> > > >> >> >>>> >> > > >> >> >>>>> just tried it, and it "does", but only if you have both >> > python >> > > and >> > > >> >> >>>>>visual >> > > >> >> >>>>> studio installed on your machine. Guess it builds the >> .dll at >> > > npm >> > > >> >> >>>>>install >> > > >> >> >>>>> time :( >> > > >> >> >>>>> >> > > >> >> >>>>> >> > > >> >> >>>>> On Thu, Jul 25, 2013 at 1:23 PM, Anis KADRI < >> > > [email protected] >> > > >> > >> > > >> >> >>>>>wrote: >> > > >> >> >>>>> >> > > >> >> >>>>>> https://github.com/rbranson/node-ffi >> > > >> >> >>>>>> >> > > >> >> >>>>>> looks like it does >> > > >> >> >>>>>> >> > > >> >> >>>>>> On Thu, Jul 25, 2013 at 10:14 AM, Filip Maj < >> [email protected]> >> > > >> >>wrote: >> > > >> >> >>>>>>> Does it work with Windows? >> > > >> >> >>>>>>> >> > > >> >> >>>>>>> On 7/25/13 10:11 AM, "Andrew Grieve" < >> [email protected] >> > > >> > > >> >>wrote: >> > > >> >> >>>>>>> >> > > >> >> >>>>>>>> Looks like the fix is pretty easy: >> > > >> >> >>>>>>>> >> > > >> >> >>>>>>>> agrieve@agrieve-macbookpro ~/git/cordova/tmp$ time >> node >> > > go3.js >> > > >> >> >>>>>>>> went 0 times >> > > >> >> >>>>>>>> went 10 times >> > > >> >> >>>>>>>> went 20 times >> > > >> >> >>>>>>>> went 30 times >> > > >> >> >>>>>>>> went 40 times >> > > >> >> >>>>>>>> went 50 times >> > > >> >> >>>>>>>> went 60 times >> > > >> >> >>>>>>>> went 70 times >> > > >> >> >>>>>>>> went 80 times >> > > >> >> >>>>>>>> went 90 times >> > > >> >> >>>>>>>> went 100 times >> > > >> >> >>>>>>>> >> > > >> >> >>>>>>>> real 0m0.444s >> > > >> >> >>>>>>>> user 0m0.266s >> > > >> >> >>>>>>>> sys 0m0.158s >> > > >> >> >>>>>>>> >> > > >> >> >>>>>>>> >> > > >> >> >>>>>>>> All I did was replace "shelljs" with "shelljs-ffi". >> > > >> >> >>>>>>>> >> > > >> >> >>>>>>>> >> > > >> >> >>>>>>>> On Thu, Jul 25, 2013 at 12:53 PM, Filip Maj < >> > [email protected]> >> > > >> >> wrote: >> > > >> >> >>>>>>>> >> > > >> >> >>>>>>>>> Cool, nice work. >> > > >> >> >>>>>>>>> >> > > >> >> >>>>>>>>> We could either try to contribute to shelljs or rip it >> > out >> > > >> >>and go >> > > >> >> >>>>>> all >> > > >> >> >>>>>>>>> child process all the time >> > > >> >> >>>>>>>>> >> > > >> >> >>>>>>>>> In any case I think this bench should be submitted to >> > > shelljs >> > > >> >> >>>>>>>>>repo. >> > > >> >> >>>>>>>>> @r2r, >> > > >> >> >>>>>>>>> dude who maintains it, would probably like to know >> > > >> >> >>>>>>>>> >> > > >> >> >>>>>>>>> On 7/25/13 9:50 AM, "Andrew Grieve" < >> > [email protected]> >> > > >> >> wrote: >> > > >> >> >>>>>>>>> >> > > >> >> >>>>>>>>>> One reason: shelljs.exec() >> > > >> >> >>>>>>>>>> >> > > >> >> >>>>>>>>>> Did a test to see how many times I could execute >> > "true". 9 >> > > >> >> >>>>>>>>>>seconds >> > > >> >> >>>>>> vs >> > > >> >> >>>>>>>>> .5 >> > > >> >> >>>>>>>>>> seconds! >> > > >> >> >>>>>>>>>> >> > > >> >> >>>>>>>>>> >> > > >> >> >>>>>>>>>> agrieve@agrieve-macbookpro ~/git/cordova/tmp$ time >> node >> > > >> >> >>>>>> shelljstest.js >> > > >> >> >>>>>>>>>> went 0 times >> > > >> >> >>>>>>>>>> went 10 times >> > > >> >> >>>>>>>>>> went 20 times >> > > >> >> >>>>>>>>>> went 30 times >> > > >> >> >>>>>>>>>> went 40 times >> > > >> >> >>>>>>>>>> went 50 times >> > > >> >> >>>>>>>>>> went 60 times >> > > >> >> >>>>>>>>>> went 70 times >> > > >> >> >>>>>>>>>> went 80 times >> > > >> >> >>>>>>>>>> went 90 times >> > > >> >> >>>>>>>>>> went 100 times >> > > >> >> >>>>>>>>>> >> > > >> >> >>>>>>>>>> real 0m8.873s >> > > >> >> >>>>>>>>>> user 0m10.941s >> > > >> >> >>>>>>>>>> sys 0m6.005s >> > > >> >> >>>>>>>>>> agrieve@agrieve-macbookpro ~/git/cordova/tmp$ time >> node >> > > >> >> >>>>>>>>>> child_processtest.js >> > > >> >> >>>>>>>>>> went 10 times >> > > >> >> >>>>>>>>>> went 20 times >> > > >> >> >>>>>>>>>> went 30 times >> > > >> >> >>>>>>>>>> went 40 times >> > > >> >> >>>>>>>>>> went 50 times >> > > >> >> >>>>>>>>>> went 60 times >> > > >> >> >>>>>>>>>> went 70 times >> > > >> >> >>>>>>>>>> went 80 times >> > > >> >> >>>>>>>>>> went 90 times >> > > >> >> >>>>>>>>>> went 100 times >> > > >> >> >>>>>>>>>> >> > > >> >> >>>>>>>>>> real 0m0.470s >> > > >> >> >>>>>>>>>> user 0m0.278s >> > > >> >> >>>>>>>>>> sys 0m0.228s >> > > >> >> >>>>>>>>>> >> > > >> >> >>>>>>>>>> >> > > >> >> >>>>>>>>>> Here's the code: >> > > >> >> >>>>>>>>>> shelljstest.js >> > > >> >> >>>>>>>>>> >> > > >> >> >>>>>>>>>> var shjs = require('shelljs'); >> > > >> >> >>>>>>>>>>> for (var i = 0; ; ++i) { >> > > >> >> >>>>>>>>>>> shjs.exec('true'); >> > > >> >> >>>>>>>>>>> if ((i / 10 | 0) == i / 10) { >> > > >> >> >>>>>>>>>>> console.log('went ' + i + ' times'); >> > > >> >> >>>>>>>>>>> } >> > > >> >> >>>>>>>>>>> if (i == 100) { >> > > >> >> >>>>>>>>>>> process.exit(0); >> > > >> >> >>>>>>>>>>> } >> > > >> >> >>>>>>>>>>> } >> > > >> >> >>>>>>>>>> >> > > >> >> >>>>>>>>>> >> > > >> >> >>>>>>>>>> child_processtest.js >> > > >> >> >>>>>>>>>> >> > > >> >> >>>>>>>>>> var child = require('child_process'); >> > > >> >> >>>>>>>>>>> var i = 0; >> > > >> >> >>>>>>>>>>> function go() { >> > > >> >> >>>>>>>>>>> child.exec('true', function() { >> > > >> >> >>>>>>>>>>> ++i; >> > > >> >> >>>>>>>>>>> if ((i / 10 | 0) == i / 10) { >> > > >> >> >>>>>>>>>>> console.log('went ' + i + ' times'); >> > > >> >> >>>>>>>>>>> } >> > > >> >> >>>>>>>>>>> if (i == 100) { >> > > >> >> >>>>>>>>>>> process.exit(0); >> > > >> >> >>>>>>>>>>> } >> > > >> >> >>>>>>>>>>> go(); >> > > >> >> >>>>>>>>>>> }); >> > > >> >> >>>>>>>>>>> } >> > > >> >> >>>>>>>>>>> go(); >> > > >> >> >>>> >> > > >> >> >> >> > > >> >> >> > > >> >> >> > > >> >> > > >> >> > > >> > >> > >
