Jump to content

Sky Slate Blueberry Blackcurrant Watermelon Strawberry Orange Banana Apple Emerald Chocolate
Photo

CMDret - AHK functions


  • Please log in to reply
26 replies to this topic
Laszlo
  • Moderators
  • 4713 posts
  • Last active: Mar 31 2012 03:17 AM
  • Joined: 14 Feb 2005
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.

corrupt
  • Members
  • 2558 posts
  • Last active: Nov 01 2014 03:23 PM
  • Joined: 29 Dec 2004
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

corrupt
  • Members
  • 2558 posts
  • Last active: Nov 01 2014 03:23 PM
  • Joined: 29 Dec 2004
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.

corrupt
  • Members
  • 2558 posts
  • Last active: Nov 01 2014 03:23 PM
  • Joined: 29 Dec 2004
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 :) .

shimanov
  • Members
  • 610 posts
  • Last active: Jul 18 2006 08:35 PM
  • Joined: 25 Sep 2005

Many thanks for the sample code shimanov :) .


I am glad I could contribute.

Concerning specifying the pipe size when calling CreatePipe:

The input and output buffer sizes are advisory. The actual buffer size reserved for each end of the named pipe is either the system default, the system minimum or maximum, or the specified size rounded up to the next allocation boundary.


and the memory allocated is from physical memory:

the system creates the inbound and/or outbound buffers using nonpaged pool, which is the physical memory used by the kernel


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.

CMDout = %CMDout% %lpBuffer% ; not as fast but allows dynamic resizing


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

DllCall("CloseHandle", UInt, hWrite)


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

corrupt
  • Members
  • 2558 posts
  • Last active: Nov 01 2014 03:23 PM
  • Joined: 29 Dec 2004
Thanks for the tips shimanov. What would you suggest to use for a pipe size?

CMDout = %CMDout% %lpBuffer% ; not as fast but allows dynamic resizing


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

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?

toralf
  • Moderators
  • 4035 posts
  • Last active: Aug 20 2014 04:23 PM
  • Joined: 31 Jan 2005

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.

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".

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.

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

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).

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.

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?

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.

toralf
  • Moderators
  • 4035 posts
  • Last active: Aug 20 2014 04:23 PM
  • Joined: 31 Jan 2005
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.

shimanov
  • Members
  • 610 posts
  • Last active: Jul 18 2006 08:35 PM
  • Joined: 25 Sep 2005

What would you suggest to use for a pipe size?


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.

Any suggestions?


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:

DllCall( "PeekNamedPipe", "uint", hRead, "uint", 0, "uint", 0, "uint", 0, "uint*", bSize, "uint", 0 )

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

corrupt
  • Members
  • 2558 posts
  • Last active: Nov 01 2014 03:23 PM
  • Joined: 29 Dec 2004
@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 :) .

toralf
  • Moderators
  • 4035 posts
  • Last active: Aug 20 2014 04:23 PM
  • Joined: 31 Jan 2005
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.

Chris
  • Administrators
  • 10727 posts
  • Last active:
  • Joined: 02 Mar 2004

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.

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):
SetBatchLines -1

StartTime := A_TickCount
Loop 500000
	var := DllCall("GetTickCount")
Elapsed := A_TickCount - StartTime

MsgBox %Elapsed%
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.