AutoHotkey Community

Version 1.03 works perfectly for me!

Toralf: I guess, inside the function you don't know the expected length of the output, so you cannot reserve the right amount of memory. Corrupt could make the output string returned, but then another parameter is needed, the max capacity. Still, you are right, it would save a couple of lines of code at calling.

Great to hear Laszlo !

Thanks for the feedback Toralf. There were a few reasons for using Byref instead of returning the string as a return value.
1) laziness... The main parts of the function were originally ported from the code used with cmdret.dll . The dll needed to return the value that way to avoid needing to use LoadLibrary and FreeLibrary calls to retrieve the value.
2) Reduce memory usage for large return values. I may be wrong with this one but the thought was that to transfer the data back as a return value, the contents of the string used in the function would need to be copied to another variable used when calling the function instead of being added to a global variable during the process.
3) Future use. There are a few other features that exist in cmdret.dll that I hope to be able to add. I'm currently thinking of adding additional functionality as optional params. to this function rather than adding separate functions. If additional functionality can be added without affecting performance and/or overly complicating things it might help give syntax suggestions for possible future integration in AutoHotkey. One example of an optional param. might be CMDerr, which could optionally separate error output into a second variable instead of the current method that merges StdOut and StdErr output. This would require a second output variable. Another example might be to output directly to an edit control rather than store the info in a variable. This could allow streaming of output to a control in realtime (close to).

Having something like Output := CMDret_RunReturn("ping 127.0.0.1") for syntax does sound appealing though and may save an additional line of code here and there... Also, the name of the function was chosen based on the name of the dll and name of the function inside the dll that was ported but probably isn't very appealing and/or easy to remember. Any suggestions?

Opinions welcome...

Edit: typos

Many thanks for the sample code shimanov . I wasn't aware of the PeekNamedPipe function. I usually use API-Guide as a reference but the version I have here doesn't list that one. I really should check msdn a bit more often... I've changed the method used to read the pipe to wait for data before trying to read from the pipe and also to wait until the process has finished and the pipe is empty to determine when to quit.

Updated to version 1.04 beta, added a link to download previous versions and added a changelog to first post .

Many thanks to everyone for testing and contributing to the development of this function so far .

Also, thanks for the congrats on the 1000th post .

I am glad I could contribute.

Concerning specifying the pipe size when calling CreatePipe:



and the memory allocated is from physical memory:



It is advisable to limit the pipe size, and to use the total bytes available, as returned by PeekNamedPipe, to specify the size of the buffer for read operations. Note that the total bytes available will vary from call-to-call, and will vary from the suggested pipe size initially specified. The best performance will be achieved by reading (and removing) all data in the pipe on each call to ReadFile.



You could always do dynamic resizing with a call to VarSetCapacity, before calling lstrcat.



This call should not be used until all pipe operations are completed (e.g., after the child process terminates).

Thanks for the tips shimanov. What would you suggest to use for a pipe size?


I was thinking that as well but the problem I can see is that VarSetCapacity clears the variable in the process so the previous data stored would be lost. I had asked Chris about an option for VarSetCapacity that wouldn't clear the contents but it seems that it likely wouldn't be much of an improvement as the memory would need to be reallocated, data moved then free the old memory. Chris had also suggested to try using RtlMoveMemory or lstrcpy instead of lstrcat if the size of the original data and size of the data to be added are known for better performance with larger strings. While this may give better performance, unless I'm misunderstanding, it would still require additional calls to VarSetCapacity to move the data from the current location then dynamically allocate more space for the resulting data size then copy the data over to the new location and free the old memory. I haven't done any further testing with this but it doesn't sound like it would end up being much faster than the current method of CMDout = %CMDout% %lpBuffer% unless an initial call to VarSetCapacity was made to estimate the size of the output data. This could eliminate the need to move the data and keep resizing. I guess a really large size could be assumed then trimmed down after the resulting size is known but I was hoping for a sky's the limit type approach that wouldn't involve having to try and guess the output size since the script could lock up and crash by guessing the size too small. Maybe VarSetCapacity could be used in stages as the size increases for better overall results? In other words, use VarSetCapacity initially and keep adding until a certain size is reached then move to a temp variable, resize to a larger size, move the data back, keep adding until the next size is reached, etc... , then trim the result at the end...?? If so, increase to what size if what size has been reached? and what would be a reasonable initial size?

Any suggestions?

I still prefer the way:
outputvar := function(params [,optional params])
This is easy to read and to remember. Even if it takes a split second more to copy the output string (which in 99% of all operations will be small in size). The function should be easy to use (preferably without needing VarCapacity). StrLen() is something that is not needed all the time and it can be achieved easily when needed, thus I consider it redundant. If I want to check if someting came out of the function, I can still check "If outputvar".

You could use/set ErrorLevel to a number or string. It would fit perfectly into the current AHK syntax.

I'm prefer to get data into vars and then put them into the GUI, separating data and gui as much as possible. And (same as StrLen()) it can be done with a single AHK command, GuiControl.

If this would be integrated into AHK, I guess it would become a "RunWait" function, thus I would suggest a simple "RunWait". Optional parameters should be identical to the AHK command, but could be extended with new once if you think they will increase usabilty.

Just my 2 cents. Sorry for my english (I tend to write very direct which some people find offensive.) I very much appreaciate your work and that of the others that contribute. Thank you all very much. I hope that it will save Chris a lot of R&D and that he will be able to introduce it into AHK.

_________________
Ciao
toralf

I just looked at the code of the first post and realized that a lot of what I have written has been done, so several things above are redundant, sorry. Thanks for the improvements.

_________________
Ciao
toralf

Since the pipe is using non-paged memory, the greatest performance hit will be incurred during script processing. I would allow the OS to manage the buffer, and specify a suggested buffer size of 0 (zero). This will suit commands which produce little to medium amount of output, and balance the memory/performance requirements for a larger amount of output.



I would leave it as is. Any modifications that result in performance improvements, would only be evident when the code is executed in greater frequency and for a larger amount of output, which is unlikely to be the norm. There should be a negligible performance hit for most jobs executed through your interface. Although, you could use "SetBatchLines, -1" to remove script delays; but, the critical path will likely be dependant on waiting for the pipe to be filled by the child process (e.g., ping command).

You should also consider that use of DllCall incurs a significant performance penalty on its own, and weigh the tradeof of using internal functions and mechanisms, versus those located in external libraries accessed via DllCall.

Most important, use "bSize" from:



to specify the requested read size in the call to ReadFile. It is desirable to minimize the number of loop iterations.

@shimanov

Thanks for the tips . I had originally thought that reading from the pipe a reasonable amount at a time might be more efficient as VarSetCapacity wouldn't have to be used between calls to keep allocating space for the buffer each time through the loop and the script's memory usage could remain fairly constant but, if I understand correctly, clearing the buffer and not letting the physical memory usage possibly increase from having a larger pipe size could have its advantages. I have made the change in version 1.05 beta.

I tend to agree that there should be a negligible performance hit with the current method used for resizing the output variable but I'm curious as to how drastic the performance hit may be for large outputs. Leaving it "as is" for now seems like a good idea though.

@toralf
Thanks for the syntax suggestions . I had originally thought of using the same syntax as RunWait and adding additional optional params. but since RunWait doesn't use the function format a few things would need to change...

I have added an option to receive the output as the Return value instead of ByRef. However, since AutoHotkey does not allow having an optional ByRef in a function, a variable will still need to be used. This could possibly be set once near the top of the script though. To specify that the output should be returned as a Return value, the variable used for CMDout should = R . When the function has finished the value of CMDout will be set to R again if this method is used so that it only needs to be set once. Please let me know what you think of this method .

Thanks a lot.
The method is ok.
If I do not want to specify the "R" I can easily remove some code segments from your function and get what I need. Thanks.

_________________
Ciao
toralf

Have you discovered this through actual benchmarks? My testing shows that performance of DllCall is very good for pre-loaded DLLs. For example, on my system the following test of 500,000 calls runs in about 1.5 seconds (i.e. three microseconds per call):

Perhaps the very first call to a given function is slower if the OS has to some kind of look-up for it (but I haven't seen evidence of that yet either).

By contrast, using DllCall on a non-resident DLL is quite slow, which is why the help file recommends using LoadLibrary if you're going to make a lot of calls to one (such as via Loop).

Compared to the fast results of DllCall above, finding the length of a string can be a very costly operation for huge strings. That’s why I suggested avoiding extra calls to it wherever possible. For example, I believe lstrcat must call something like lstrlen internally, which would be quite wasteful if you already know the length of the first/base string. In such a case, calling lstrcpy (or better: RtlMoveMemory if you also know the length of the second string) should be much faster, though I haven't actually benchmarked it in a script.

I have not. Although, I have seen the code for DllCall and associated functions. My comment refers to the relative performance of calling internal commands versus their equivalent located in an external library.

I advise to "weigh the tradeof" of using each mechanism, when performance is an issue.

It still works, almost perfectly (version 1.05). A few questions:
- What do you use the GLOBAL cmdretPID for? When the function returns, it is empty. Do you want to check its value in a timer routine?
- StringTrimLeft, CMDout, CMDout, 2 cuts off the first two characters of the output. Did this instruction remain from an earlier version?
- SetBatchLines, -1 in a function makes a global change. You probably want to save the old value beforehand and restore it before return, but it is easier to remove it, and let the user specify it outside of the function, if he needs that extra speed.
- Do you need VarSetCapacity(lpBuffer, 0)? This local variable is removed, when the function returns, anyway.
- Do you need " OR bSize > 0" in the instruction IF (ErrorLevel OR bSize > 0)? The next line checks if bSize > 0, anyway.
- To preserve any leading or trailing spaces, don't we need AutoTrim Off?

Here is a variant, without the ByRef parameter and with changes according to the list above. I also made it more flat, such that there are fewer levels of nested if's (it is easier to find the matching braces). I made it for myself, to be able to understand the logic of the 1.05 code.

For me (WinXP SP2) the first messagebox seems correct, but the 2nd (from ("cmd /c dir c:\")) is blank.