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 <lewis@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