You can change something simply by observing it!

[David Kenny]

Greetings,

I know this must not be the proper way to do this but I am stumped. The following code loops through a treeview branch and get the description for each computer. When a compute is not present on the network, it takes much longer for the routine to return. So I am using threads. There might be 300+ machines to query, so I don’t want to start more than nine threads at a time. The WHILE loop is meant to keep the thread count less than ten. Now a couple scenarios:

1. Without the WHILE loop at all, the code works as planed. It works better, however, on a branch with <50 or so items as there is no limit on the number of threads started.

2. With the WHILE loop as it is shown, the program appears to sleep forever (even if the SLEEP is commented out. It has to be stopped as it won’t respond to a close.

3. With the WHILE loop as it is shown, and with the MSGBOX line uncommented, it works as it should. Limiting the threadcount, but obviously making me close each msgbox as it pops up. The program behaves normally and will shut down when told to close.

Function FORM1_BN_CLICKED ( _
                                           ControlIndex     As Long,  _  ' index in Control Array
                                           hWndForm         As Dword, _  ' handle of Form
                                           hWndControl      As Dword, _  ' handle of Control
                                           idButtonControl  As Long   _  ' identifier of button
                                           ) As Long
    Local i				As Long
	Local hTreeItem		As Long
	Local hChildItem	As Long
	Local sComputer		As String
    hTreeItem=TreeView_GetSelection(HWND_FORM1_TREEVIEW1)
    If FF_TreeView_GetLevel (HWND_FORM1_TREEVIEW1, hTreeItem)=2 Then 'Pointing to a computer
    	hTreeItem=(TreeView_GetParent(HWND_FORM1_TREEVIEW1,hTreeItem)) ' get its parent
    End If 
    hChildItem=TreeView_GetChild(HWND_FORM1_TREEVIEW1,hTreeItem)
    While hChildItem<>%Null
    	'sComputer=FF_TreeView_GetText (HWND_FORM1_TREEVIEW1, hChildItem) 
         ' the above line was commented out because it wasn't being used and was therefore misleading
	    While ThreadCount>9 
	    	Sleep 200 
	    	'msgbox str$(threadcount)
	    Wend
	    Thread Create get_description(hChildItem) To hTreeItem
	    hChildItem=TreeView_GetNextSibling(HWND_FORM1_TREEVIEW1,hchildItem)
    Wend 
End Function

Regards,

David

[Kev Peel]

Define the "h"-prefixed handles as DWORD, that might help. I see you're using 3rd party library code (FireFly I think??), so this may be a question for the author.

[Michael Mattias]

hTreeItem=TreeView_GetSelection(HWND_FORM1_TREEVIEW1)
.....

Thread Create get_description(hChildItem) To hTreeItem

Ok, I give up: is 'htreeitem' a handle to a treeview item or a handle to a thread of execution?

MCM

[Chris Boss]

First, the use of many threads will not speed up your code, but will actually slow it down. Each thread has overhead and takes away CPU time.

You only need one thread for the entire routine, so it does not slow down the main GUI thread (primary thread). If your code needs to be recursive, simply use a function for that, not a new thread.

Next, don't use the library routines which use both form handle and control ID. This makes repeated calls to an API function required to convert a form handle and control ID to the controls handle.

For speed sake, it is better to get the controls handle once and then make calls using SendMessage (or a simple wrapper function) which only requires the control handle.

The point is, if speed is your concern, then decrease the overhead of the API calls used.

[David Kenny]

Kev,

Thanks for the suggestions, but neither of those would explain why taking the WHILE loop out would change anything. No FireFly functions in there and not using the two variables in there either.

[Michael Mattias]

>The WHILE loop is meant to keep the thread count less than ten.

See: Semaphore Objects

BTW, using separate threads of execution may do no good: Depending on how the FF_GetTreeText (or whatever the name, I can't see the other posts in 'edit' mode) function works, the software may have to switch to the thread owning the window anyway.. resulting in... exactly the same sitution as though it were a single thread of execution.

Wait a minute...

When a compute is not present on the network, it takes much longer for the routine to return

That makes no sense, based in the code posted, which is doing nothing but retrieving the text of treeview nodes.
Something you are not telling us here?

[Roger Garstang]

Semaphore or something using an InterlockedIncrement/Decrement is the best way. I never had much luck getting ThreadCount to do what I wanted. Also, instead of Sleep, use the FireFly DoEvents function in a tight loop, it works much better...or use one of the Wait API Functions since you will want a graceful way to exit the threads too on aborting or Close being clicked (Checking for the Window existing or other GUI states or global variable flags for if it is running don't work the greatest...although I usually use a variable called Running myself where close only works if it is 0 and if it is 1 Close aborts and sets it to 2 which tells my thread to exit which sets it to 0 so Close then works. Wait functions are better for multiple though. Then you can use them to delay and to check for abort/close.

Unless you have a really slow network where the requests take a long time, the spawning of that many threads may not be worth it too. Maybe just 3-4 would be better. Although I've seen Explorer open 9 up to do a file search.

I also don't see where you are closing your threads:

Thread Create ServerThread(hServer) To hSrvThread
Thread Close hSrvThread To hThreadReturn

You can close them right away, it just closes the handle out and the thread still executes. Without closing them Threadcount is most likely always returning high numbers since they were never closed. Msgbox is only making it appear to work since by the time you close it it delayed enough that the previous threads are finished...although still not closed, so? Like I said above, I don't use ThreadCount, so The specifics on what it returns if the threads aren't closed I don't know at the moment. I do know there is a delay on its count which is why I didn't use them.

[Roger Garstang]

BTW, using separate threads of execution may do no good: Depending on how the FF_GetTreeText (or whatever the name, I can't see the other posts in 'edit' mode) function works, the software may have to switch to the thread owning the window anyway.. resulting in... exactly the same sitution as though it were a single thread of execution.

Wait a minute...

That makes no sense, based in the code posted, which is doing nothing but retrieving the text of treeview nodes.
Something you are not telling us here?

I believe his thread get_description is getting the network info. You are correct in that multiple threads would still lock somewhat and not allow the treeview to respond very quick which is why I suggested he use DoEvents (A FireFly Function to process current GUI messages) so the GUI isn't froze. A network connection that isn't there does take longer to respond since it waits the timeout. If he is just "pinging" it and/or using PB's TCP functions he could decrease the timeout. Or if he is using API they may have a timeout period, or he could Ping the PC before the API call.

[David Kenny]

Chris,

Thanks for your response.

I pull a list of machines from Active Directory (some are up and running and some are not). The reason for the thread is to keep busy contacting machines that are present while not having to wait for the routine to return after timing out because it didn't find a machine. It would only slow down if all 9 other threads were each trying to contact a machine that isn't there. Using the extra threads is speeding it up quite a bit actually. But I am limiting it to 9 to cut down on the overhead as you mentioned.

Point taken on the Control ID's. Not an issue here though.

Michael,

You caught me cheating. The thread closes itself as before it exits. I didn't need that handle so I dumped it in a variable that wasn't being used at that time. Just a blatant example of why doing stuff like that makes for unreadable code. It will never be read by anybody other than me. (unless I post it on a website :doh

[Roger Garstang]

Michael [Roger],

You caught me cheating. The thread closes itself as before it exits. I didn't need that handle so I dumped it in a variable that wasn't being used at that time. Just a blatant example of why doing stuff like that makes for unreadable code. It will never be read by anybody other than me. (unless I post it on a website :doh

How is the thread closing itself before it exits? Your thread handle is given to a local variable, so the thread has no handle to close...even if it did go to a static or global the next thread is assigned to the value and it would be invalid.

[David Kenny]

Michael,

Roger was right. The code to contact the machine is in the thread. I was going to pass the name of the machine to the routine and grabbed the name to do that. When I remembered that I couldn't pass a string to a thread directly, I abandoned that idea in favor of getting it from the TV directly in the thread code by passing the handle to the thread item itself. So, I forgot to remove the code that gets the text from the TV. Updating it now.

Thanks for the tip about Semaphore's I will look into that. And the tip about pinging the machine first, that should speed it up quite a bit.

David

[David Kenny]

Roger,

How is the thread closing itself before it exits?

Thanks so much. You solved it. I have another worker thread that does it the way I described this one working. I forgot to make this thread work that way. I will now have to pass a UDT to the thread as I do with the first one.

Regards,

David

[Michael Mattias]

I just did a quick search on 'semaphore', all forums, message subject, forever.

There are only two message (2) threads, neither of which includes a nice demo.

Any volunteers?

MCM

[Roger Garstang]

I used one in my SearchXP app that was to replace XP's search. It kind of got put on hold, but many threads have been asking about a search with the exact features it has. I use it to limit the threads that spawn for each subfolder to 16...I'd probably make it half that now, but at the time it was the quickest number, after that it would actually go slower.

I just create the Semaphore with:

Instance= CreateSemaphore(ByVal 0, 16, 16, ByVal 0)

Then WaitForSingleObject on Instance and when 16 are used it Waits.

When each thread is done I call:

ReleaseSemaphore(Instance, 1, tmpReturn)

to increase the count again.

Pretty simple and much better than the InterlockedIncrement/Decrement calls.

[Michael Mattias]

>The thread closes itself as before it exits

Semantics, Semantics, Semantics!

A "thread" does not "close."

An open thread handle may be closed. A thread function may complete. A thread object may be signalled.

"Thread" should be an adjective!!!

MCM