On Tue, Oct 14, 2014 at 07:19:10AM +0200, Thierry Goubier wrote: > Hi Dave, > > took me a while to do some testing. It seems there is an issue, but I > couldn't find out where. First, the data: > > Loading a package from a local gitfiletree repository (get from the > command a zipped git archive containing the package) : > > 4.5.11, 4.6.8: 2132 msec. > 4.5.12, 4.6.9: 50847 msec. > > I tried to rewrite the main command, remove the closePipes, etc... No > change in the runtime, so your right. > > Profiling doesn't help much: execution time is mostly idle time (but > with a significant increase on PipeableOSProcess commands: goes from > invisible to 2700 ms. A user break in the middle shows it delay waiting > on output of the external process). > > Only hint I could see is that it seems to be linked with the size of the > output of the command.
Thierry, Aha! I am quite sure that you have located the problem. There must be something preventing the OS process from running to completion with a larger amount of output data. It definitely sounds like a bug in my latest update to Pipeable OSProcess. Thank you very much for finding this. I will get it fixed as quickly as I can. Dave > > Le 11/10/2014 15:34, David T. Lewis a ?crit : > >On Fri, Oct 10, 2014 at 01:32:27PM -0400, David T. Lewis wrote: > >>>Le 10/10/2014 17:52, David T. Lewis a ?crit : > >>>>>2014-10-10 14:09 GMT+02:00 David T. Lewis <le...@mail.msen.com>: > >>>>> > >>>>>> > >>>>>>Right. But please do test it in your applications to be sure, I really > >>>>>>only > >>>>>>did simple testing and I am sure there may still chances for problems > >>>>>>in > >>>>>>this area. > >>>>>> > >>>>> > >>>>>Hi Dave, > >>>>> > >>>>>Would it has a chance of slowing down things a lot? > >>>>> > >>>>>There is apparently something going very very slow compared to > >>>>>OSProcess > >>>>>4.5.11 when used from GitFileTree. So slow that I killed the image > >>>>>building > >>>>>script before it was over. Reverting GitFileTree to 4.5.11 solved it. > >>>>> > >>>> > >>>>I don't this so, but I am not certain. The update for PipeableOSProcess > >>>>affects only the methods in PipeableOSProcess class>>command: and > >>>>closely > >>>>related methods. If GitFileTree is using that idiom, then it is > >>>>certainly > >>>>possible that I have introduced a bug that does not show up in my unit > >>>>tests. > >>> > >>>Typical code in GitFileTree is this: > >>> > >>>[ > >>>c := PipeableOSProcess command: ''. > >>>output := c output. > >>>... > >>>] ensure: [c closePipes] > >>> > >>>Maybe it's triggering something. > >>> > >>>Thierry > >>> > >> > >>Hmmm... I wonder if the #closePipes is causing a problem now. I don't know > >>if I ever tested to see if sending closePipes works after the pipes are > >>already closed, and my recent change closes the pipes after #output is > >>evaluated. I'll check it when I get home. > >> > > > >I tried this in both Squeak trunk and Pharo 3.0: > > > > (1 to: 100) collect: [:e | | c output | > > [c := PipeableOSProcess command: 'ls -lt'. > > output := c output. > > ] ensure: [c closePipes]. > > output ]. > > > >I get the expected results, and no open file handles left over. > > > >The #closePipes is no longer needed, so this can now be simplified as > >follows, > >which also leaves no open file handles: > > > > (1 to: 100) collect: [:e | (PipeableOSProcess command: 'ls -lt') > > output]. > > > >I am not sure what the issue is when used with GitFileTree, but it does not > >look like the change to PipeableOSProcess class>> command: is directly > >involved. > >But ... I'm probably missing something. > > > >If you are fairly sure the that problem is related to the the > >PipeableOSProcess > >change, then the only other thing I can think of right now is that there > >may > >be some other places in GitFileTree where PipeableOSProcess is being used > >in > >a different way, and maybe I caused a problem there due the the change in > >the > >#command: methods. But this a a complete guess, I really do not know. > > > >Dave > > > > > > >
