I take that back, at least partially ;).

Try runing rar to compress some huge file, and then try your test app at the same time.

It took shell_ex 1 minute and 40 seconds to finish 10 calls, whereas wshell took 800 ms.
Posted on 2005-05-07 10:03:30 by Jibz
Your testing method is defective, running PKZIP on a 75 meg VOB file, I get about the same variation between the two algos, both run at about 14 seconds.

Changes to the demo file are as follows.


            fn shell_ex,"pkzip kari.zip kari.vob",NORMAL_PRIORITY_CLASS

            fn wshell,"pkzip kari.zip kari.vob"


Degradation percentage is zero point zero.

LATER : I removed the loop to get single timings on a single LARGE file.
Posted on 2005-05-07 10:20:49 by hutch--

This runs rar and lowers it's priority while compressing, which is a nice way to run large backups without having to leave the computer alone in the meantime.

... try again, hutch--.
Posted on 2005-05-07 10:23:11 by f0dder
Read NORMAL_PRIORITY_CLASS.

I am not interested in your opinion and the rules are not either.
Posted on 2005-05-07 10:26:12 by hutch--
Since when is NORMAL_PRIORITY_CLASS a lower than normal priority?

Jibz' method of testing is not flawed. He runs a compression at a lower priority, which is a useful and often-used thing to do when you're compressing large amounts of data and want to do other work on your box at the same time.
Posted on 2005-05-07 10:33:49 by f0dder
I ran PKZIP which is a common program for archiving and there is no difference. It is a compressor that runs at the same time as the test app so the theory is wrong and the test is defective.

For the theory to be correct that GetExitCodeProcess() slows down other processes of whatever priority, it would show when running BOTH pkzip and the test app. There is no difference in the timing, both run at 14 seconds. Proof delivered.

Note again that I am not inerested in opinion and neither are the rules.
Posted on 2005-05-07 10:39:36 by hutch--

I ran PKZIP which is a common program for archiving and there is no difference. It is a compressor that runs at the same time as the test app so the theory is wrong and the test is defective.

So, by using a completely different program, running at normal priority while the point is that *lower* priority threads are slowed down, you believe you've disproved there is a performance problem?

Jibz clearly showed the problem. Posted reproducible information, with a 1sec->1min55sec slowdown as a result.

Like it or not, lowering priority on a big compression task is pretty normal, since it allows you to use your computer at the same time, without any perceptible slowdown. Before WinRAR had the ability to lower it's priority, I used taskmgr to do it manually...

Posted on 2005-05-07 10:50:33 by f0dder
The rules require proof, not opinion. I have delivered proof that the polling loop using GetExitCodeProcess() is not slower. The test I have used is a perfectly reasonable test using a processor intensive task running at the same time as the test app. There is no difference in the times.

Proof delivered.
Posted on 2005-05-07 10:56:56 by hutch--
Jibz delivered proof of the slowdown - and it's obviously quite massive.

It's perfectly reproducible as well - "rar a -ri1 out.rar ..\*.zip" executes within a second with cputestw. Running cputest, I terminated the compression after a minute.

Proof delivered.
Posted on 2005-05-07 11:09:17 by f0dder
So is this.


            fn shell_ex,"pkzip kari.zip kari.vob",NORMAL_PRIORITY_CLASS
            fn wshell,"pkzip kari.zip kari.vob"


Massage a test hard enough and you can get any result you like. Start altering priorities and you have changed the task being run. For the view that both yourself and Jibz have failed to sell that GetExitCodeProcess() slows down external processes to be correct, it would show while running another task (pkzip) but the times prove that it does not. Also note that with the code for the algo published at www.masm32.com that the priority of the calling app using the shell_ex proc is set to THREAD_PRIORITY_IDLE so it is doubtful that an idle priority thread can effect another task to the extent claimed.

The only proof that has been delivered on Jibz's part is that altering thread priority alters the thread priority.

As you have not contribued to the debate apart from nuisance value and endless opinion, take note of the rules that SpooK has put in place regarding your opinion without backing..
Posted on 2005-05-07 11:25:05 by hutch--

As you have not contribued to the debate apart from nuisance value and endless opinion, take note of the rules that SpooK has put in place regarding your opinion without backing..

I've backed my claims - refer to the thread_priority tests I've submitted. Jibz' rar example is 100% reproducible, and I've confirmed it.


Massage a test hard enough and you can get any result you like

Which is exactly what you've been doing - ignoring the samples I've provided, claiming Jibz' result is invalid, and fudging around with a bunch of different shell routines to hide the fact that the original shell has bad performance.
Posted on 2005-05-07 11:33:21 by f0dder
Attachments:
Posted on 2005-05-07 12:25:01 by f0dder
... easy now ;).

I'm sorry I left the discussion where I did, but I had to leave to visit some family for the weekend. I am on dialup now, so I will only post a quick note.

I'm a bit sorry I posted the comments about shell and shell_ex so close to each other, as it seems to have brought about a little confusion :|.

The cpu-related problem with shell is that it takes cpu cycles away from programs when doing it's loop .. the example for this was running a program at lower priority like rar background archiving or some similar background task (my virus scanner showed the same issue).

The problem with shell_ex is that while lowering the priority of the waiting thread does free up cpu time for other tasks including background tasks to run, it means that the waiting thread won't get scheduled again until the OS gets around to it. The example for this was to run rar at normal priority on the side.

Now, the problem with using (pk)zip instead of rar is that zip is quite i/o bound because the compression is so simple and fast, and the buffer size is so small. This means the waiting thread in shell_ex will get scheduled every time zip performs i/o, and thus it can be hard to tell the difference. You need something a bit more cpu heavy like rar or aPLib.

The two functions really just shift the problem forward and back, but do not solve it in general as the wshell function does (which is basically identical to what f0dder suggested).

I am happy to see the wshell function is in M32LIB, although I do really think there should be a version that checks the success of the CreateProcess call as well.

But I think it's a bit problematic that the shell function is listed as the default choice, and with no warnings as to the potential problems in it's documentation page. It works fine if you only run it a limited number of times, and the programs you run finish quickly, but people should be aware of the behaviour under other circumstances.
Posted on 2005-05-07 14:42:19 by Jibz
Source code is by f0dder. Posted in accordance with the rules. f0dder has claimed that this is a "generic" routine that should be used instead of any of the shell based routines in the MASM32 library.

Defect one (1)
For the routine to be generic code (defined as being able to be used in every circumstance) it has un-necessary redundant code in the form,

invoke GetStartupInfo, addr st_info


The vast majority of Windows applications that can be called with this routine never require the structure to be filled with any values so this code is un-necessary.

Defect two (2)
For the routine to be generic it would need to be able to set the priority of the called app to a user defined value.


shell proc cmdline:DWORD
LOCAL st_info:STARTUPINFO
LOCAL pr_info:PROCESS_INFORMATION

invoke GetStartupInfo, addr st_info

invoke CreateProcess, NULL, cmdline, NULL, NULL,
NULL, NULL, NULL, NULL, ADDR st_info, ADDR pr_info
test eax, eax
jz @@out

invoke WaitForSingleObject, pr_info.hProcess, INFINITE

invoke CloseHandle, pr_info.hProcess
invoke CloseHandle, pr_info.hThread

@@out:
ret

shell endp
Posted on 2005-05-07 17:55:10 by hutch--
Let's see. Reasonable example showing defect is posted. Hutch ignores, and tries a smokescreen diversion.


Defect one (1)
For the routine to be generic code (defined as being able to be used in every circumstance) it has un-necessary redundant code in the form,

It might be superfluous in most situations, but it works - in every circumstance. Also, this is an example of choosing size over speed, since the overhead of GetStartupInfo is unnoticable even if you were running on a 486.


Defect two (2)
For the routine to be generic it would need to be able to set the priority of the called app to a user defined value.

The routine was designed to have the same interface and functionality as the original shell, so this is an invalid point.


It is fair to say in this context that the first instance of f0dder.lib shows poor procedure design and a lack of understanding of the considerations involved.

I'm not doing a "f0dder.lib" and I've never claimed that I would. The issue is that masm32libs implemetation of shell is broken, and I have shown a fix with the same interface and functionality.

If you review my previous post, http://www.asmcommunity.net/board/index.php?topic=21100.msg159809#msg159809 , that is a clear and unambiguous example of the shortcoming of the masm32lib shell function.
Posted on 2005-05-07 19:23:13 by f0dder
To help resolve the primary issue, I have to ask both of you to provide the most common, realistic, and relevant examples of quality code (i.e. the way it should be done). Don't post examples that may be causes of poor programming, I personally do not wish to promote poor programming methods in this community.
Posted on 2005-05-07 19:31:40 by SpooK
SpooK,

This issue cannot be resolved as the basic ground rules cannot be established. I publically invite you to close this thread so that the forum is not used as a vehicle for personal attacks of the type that has continued in here for years. I will happily resolve the issue elsewhere.

Regards,

hutch at movsd dot com
Posted on 2005-05-07 20:12:10 by hutch--
The original subject was that masm32lib shell functions are broken, and that the idea was to show *why* they are broken and how to fix it. That's why I stayed with the original interface and functionality of "shell".

If I need something roughly equivalent to the ANSI C system() function (which the original shell() appears to have been modelled after), I use something very similar to the fixed shell() routine I've shown above.

If more flexibility is required than what system() offers, I tend to need a lot of additional flexibility, and directly use CreateProcess. The following code is a generic system() implementation, with proper error checking and resource freeing.


;
; DWORD system(char *cmdline) - roughly equivalent to ANSI C system() call.
;  input:  cmdline = command line to execute
;  output: -1 on error, otherwise process return code
;  remark: re-uses st_info.cb field to avoid extra local storage and cuts
;          down slightly on instructions - mostly done because it's cute.
;
system proc cmdline:DWORD
    LOCAL  st_info:STARTUPINFO
    LOCAL  pr_info:PROCESS_INFORMATION

    ; the st_info.cb initialization should be superfluous, as PlatformSDK says
    ; the lpStartupInfo is OUT only. But better safe than sorry...
    mov    st_info.cb, sizeof st_info
    invoke  GetStartupInfo, addr st_info

    invoke  CreateProcess, 0, cmdline, 0, 0, 0, 0, 0, 0, ADDR st_info, ADDR pr_info
    mov    st_info.cb, -1
    test    eax, eax
    jz      @@out

    ; wait for process termination
    invoke  WaitForSingleObject, pr_info.hProcess, INFINITE

    ; re-use st_info.cb field instead of adding a local var
    invoke  GetExitCodeProcess, pr_info.hProcess, addr st_info.cb
    test    eax, eax
    jz      @@out

    ; handle cleanup to avoid kernelmode object leak
    invoke  CloseHandle, pr_info.hProcess
    invoke  CloseHandle, pr_info.hThread

@@out:
    mov    eax, st_info.cb ; process returncode or -1
    ret
system endp

ENTRY32:
    invoke  system, CTEXT("blah")
    invoke  system, CTEXT("notepad")
    invoke  system, CTEXT("notepad.exe c:\test.txt")   

    invoke  ExitProcess, 0

END ENTRY32

EDIT: - tabs to spaces, since I use 4space tabs.
Posted on 2005-05-07 20:17:03 by f0dder
I again note that the topic as it has been addressed is not a technical subject but fits into the class of "other", one I am willing to resolve elsewhere so that this forum can be used for its original purpose, assembler programming.

The criticism has been going for a few years on the same basic premise yet there has been nothing new for over two years in the suggestions. To summarise, the original "shell" procedure does not close 2 handles and it is obvious in the source code noting that the source code is published with the MASM32 prject and it is targetted at assembler programmers who actually CAN read assembler code.. I have long ago explained why this procedure is done this way, complete with the leak of the two handles. It is designed as a fail safe DOS emulation procedure that runs on hadware that the suggested replacement crashes on.

I have published the processor type, an AMD K6-2 3D NOW running at 550 meg. This processor is know to cause problem with Microsoft operating systems to the extent that Microsoft have published an errata and a patch to try and fix the problem of it crashing with win95b installed on it.

The following URL is proof that operating system defined code is not garranteed to run on all hardware.

http://www.microsoft.com/windows95/downloads/contents/WURecommended/S_WUServicePacks/AMDPatch/Default.asp

The original "shell" procedure succeeds in what it was written for, to be "fail safe" on a massive range of hardware and this is proven by it not failing on hardware that the suggested replacement does fail on. I get substantial feedback on some of the most obscure problem with people installing the MASM32 project on different hardware with diferent language versions of Windows and different versions of Windows.

Among the range of variations are virus damaged machines, machine with other hardware problems, machines with damaged system DLLs and the like yet the original "shell" procedure works on all of them so far. While I have had feedback on many other problems over the years, I have never had any over the operation performed by "shell".

I have made the point some years ago when this issue was first raised that the original shell procedure will NEVER BE MODIFIED as I need its fail safe characeristic for exactly what I use it for, the masm32 installation.

The issue of repeatedly raising this criticism is not technically based, it has been known for years for exactly the reason I have mentioned above, it must work where other will crash. The repeated raising of the SAME issue has motivations that fit into the class of "other". To save the noise and nonsense that will continue to be posted in here I have offered Spook the opportunity to close the thread and I will happily deal with it elsewhere were the noise does not matter.
Posted on 2005-05-07 20:56:03 by hutch--
I guess it is resolved then. Since f0dder cannot convince hutch on how to fix a possible problem and hutch is convinced that there is no problem, no matter who is right or wrong, then I see no reason to continue the discussion. Thread closed.

Posted on 2005-05-07 21:00:21 by SpooK