Re: Strange Behavior

From: BRM (BRM_at_discussions.microsoft.com)
Date: 09/06/04 

Date: Sun, 5 Sep 2004 21:17:02 -0700

Well...let me explain a little more about my "situation"

1) I recognized over a month ago that the program would be having some major
design issues in this version. However, budget does not allow for fixing of
those issues in this version, so I have to put up with them and work around
them. Budget is being set aside in a later version to actually fix some of
these design problems.

Note: Most of the design problems came about because of a lack of
understanding of MS's way of doing things, and then learning too far in how
to do it better.

2) As to the Queue issue & searching it, there is only one server this is
for. The states represent what the client wants to do. Each item will
eventually process in a matter of minutes to hours, and there will only ever
be two threads that will be processing this information. The threads were
created because of a blocking situation with a function. Functions that would
result in a blocking state have a thread spun off to handle that blocking
state and return appropriately. A simple solution.

Creating entire CWinThread objects would be far too burdensome for the task,
so a simple function is created instead. Good programming practice (and away
from global variables) dictates other things have to be done certain ways.
So, overall, there is not a terrible amount I can do at the moment to solve
some of the issues.

Truly, I would like to call the current version my 'scrap' version (in
keeping with good S.E. and figuring you will always throw away the first
version or two), but as I said, our budget doesn't allow for it at the moment.

I think I covered everything, but I'm not positive. (It's late at night and
I need sleep.)

Thanks for the suggestions and I will bring them up when I can.

BRM

"Joseph M. Newcomer" wrote:

> Thread creation is expensive. Much more expensive than thread activation. So I'd suggest
> using a better approach than creating a thread each time an item appears. Thread creation
> can certainly impose discernable performance penalties over simple thread activation.
> Thread pooling (keeping pre-created threads around) is one of the ways of getting
> SIGNIFCANT performance improvement. Microsoft has, internally, a "threading team" that
> helps application developers understand threading, and I saw a presentation at one of the
> MS conferences by one of these people on how to maximize threading performance. The first
> rule was "don't create threads very often". Another was "if they are compute bound, don't
> create more threads than CPUs". Another was "if they are not compute bound, don't let them
> block very often", for example, use asynchronous I/O instead of synchronous I/O, because
> the scheduler overhead would be a killer.
>
> The whole thread-creation/timer-polling model sounds really bad from an architectural
> viewpoint. Any time you have a race condition as you describe, the problem is solved by
> fundamental rewrites, not tossing another timer at the problem. You sound like you're
> trying to "debug to correctness", a usually bad, and almost always terminal, approach.
>
> Dialog creation is also expensive; I would be tempted to do as I have done in the past
> where I create a modeless dialog, and merely show/hide it as required. Not sure why you
> would find the reuse a problem.
>
> Instead of using a single queue with multiple statuses, it sounds like it would make more
> sense to have multiple queues. The multiqueue model is known to give overall better
> performance than the single-queue model when there are multiple servers involved, and
> conceptually, each status represents a different server, as I've understood the problem.
> Otherwise they would not need to search the queue.
>
> Put it this way: suppose you had a bank in which you wanted to have an "express" service:
> one deposit or withdrawal, no cash deposits, and a general service for everything else.
> Which would make more sense: to have a single line, where, when a teller was finished with
> a transaction, he or she came around to the front of the line, and started down asking
> each person in turn "do you have a single transaction, deposit or withdrawal"?, then
> taking that person away, unlocking a private office, sitting the person in the chair, and
> summoning yet another teller to handle the transaction, or having the "express" line where
> people with one transaction lined up and were handled in FIFO order? The structure seems
> unnecessarily baroque, given how well-understood queueing models are. You should never
> need to search a queue for a suitable entry. That alone is very suspicious. Programs that
> don't seem to work well unless various timers are set up in certain specific ways are
> deeply suspect. Polling is always suspect. Thread creation is something that should take
> place infrequently; for example, I spin off a thread when I get a network connection, and
> that connection is expected to remain in place for hours. The cost of the thread creation
> is irrelevant. If I was trying to process 10,000 transactions per minute, I would never
> consider an architecture other than thread pooling to handle something of this rate, and
> if I was really concerned about performance, I might consider fibers as an alternative,
> but I'd never consider anything with dialog or thread creation on a per-transaction basis.
>
> You have essentially chosen worst-possible-scenarios for the implementation, using very
> heavy-duty operations, apparently frequently.
>
> I think it is time to rethink the fundamental design, instead of patching and kludging in
> the hopes that it can be made to work.
> joe
>
>
> On Fri, 3 Sep 2004 09:45:03 -0700, BRM <BRM@discussions.microsoft.com> wrote:
>
> >That sounds like it would work if that was how the algorithm was designed. In
> >the case I have, the transfer is handled by a single object that starts a
> >worker thread (and only a single worker thread) to perform a transfer and
> >another to perform status updates to the GUI. (This object is where all the
> >locking is done to ensure the thread safety.) These two threads (which are
> >the only extra threads in the system at the moment) only exist when there is
> >an item to transfer.
> >
> >Could that kind of design still cause what you are describing? I'm not sure,
> >but I would lean towards not as there is no polling, just updating.
> >
> >On the other hand, I have two timers polling - and this could be the real
> >issue. Both timers poll, however, using a different function that only counts
> >the number of items with a given status. Each performs two polls as they both
> >search for different states. Each timer is associated with a different
> >dialog. One runs ever 500 ms, and the other 250 ms.
> >
> >The only reason I did not consider this before is that the problem exists
> >even with only one timer running if I start a transfer before the other
> >dialog is created. (Besides my main dialog, the other dialogs exlicitly exist
> >only when needed. So they may be created and destroyed multiple times during
> >the program, not simply made hidden when unwanted and visible when wanted.)
> >
> >TIA,
> >
> >BRM
> >
> >"Joseph M. Newcomer" wrote:
> >
> >> OK, that's useful information. The problem has nothing to do with adding a single call
> >> level. There is something deeper going on here, and much more fundamental. The extra level
> >> of call, on a gigahertz machine, adds at most a couple nanoseconds. On a really bad day,
> >> it might add a whole memory cycle, so you might end up adding, occasionally, as much as
> >> 10ns or so.
> >>
> >> When something like this happens, I tend to suggest setting a breakpoint and doing some
> >> single-stepping. Usually this uncovers the error as you suddenly see where thnigs went
> >> wrong.
> >>
> >> You do have a profiler: QueryPerformanceCounter. A bit clumsy to use, but it will give you
> >> performance data. But when you see a problem like this, single-stepping is usually the
> >> most effective solution.
> >>
> >> For example, one of the common problems when threading is used is to have a second thread
> >> pollilng. It eats the machine alive. Therefore, it would always be inappropriate to have a
> >> second thread poll the state of the queue; it should block on a semaphore if the queue is
> >> empty. That's what I suspect your problem is.
> >>
> >> In a properly-designed queueing system, no thread ever runs unless there is something for
> >> it to do. Asking the queue if there is something to do by examining queue status is
> >> something to do. Waiting for something to appear in the queue and blocking on the
> >> semaphore means there is nothing to do, and the thread blocks. You have all the symptoms
> >> of a polling thread, and a level of indirection will not solve this. A redesign of the
> >> algorithm will.
> >>
> >> The question appears to be unrelated to the design of MFC.
> >>
> >> Given the design is "entirely OO", it should mean that any attempt to read a queue for
> >> which there is nothing to do should block. And that in general this abstraction should be
> >> invisible to the application. "Nothing to do" means "the queue is empty".
> >>
> >> Read my essay on semaphores on my MVP Tips site. Your problem appears to be neither C++
> >> nor MFC, but algorithm design.
> >> joe
> >>
> >>
> >> On Fri, 3 Sep 2004 06:35:06 -0700, BRM <BRM@discussions.microsoft.com> wrote:
> >>
> >> >The Queue is not being used between threads but to store information for
> >> >transferring. The status bears the state of the item.
> >> >
> >> >There is more than one thread that accessses the queue, but the interface to
> >> >the second thread is synchronized appropriately. There are only two threads -
> >> >a main thread, and a modifier thread. The main thread does everything to the
> >> >queue, while the modifier thread will modify a status and another variable to
> >> >the queue item itself, and then notify the main thread when an item needs to
> >> >be removed.
> >> >
> >> >As to the "grinding to a halt", if I use the implementation given, then the
> >> >program runs so slowly when there are items in the queue that it makes the
> >> >program useless. The program does still run - it does not break, produce a
> >> >bug, or anything else, it just runs extremely slowly.
> >> >
> >> >At this point in time, we do not have time to move the queue into its own
> >> >thread, but that will be done eventually. The model will probably change a
> >> >bit when the queue is spun off to its own thread.
> >> >
> >> >
> >> >As to what you were saying about the wrapper, that is exactly my point. I
> >> >want the function to exist in the wrapper alone, but the performance produced
> >> >in doing so, at this point, makes it impossible to do.
> >> >
> >> >I think a profiler might help to solve the issue in this case, but I don't
> >> >have one, nor do I know of a good free one. So suggestions are very welcome.
> >> >
> >> >As to my question here, I am asking why would a single function call level
> >> >turn a program that responds well into a program that it too useless to use?
> >> >
> >> >I.e:
> >> >
> >> >int Status() { return Queue_Status(); }
> >> >...
> >> >Item_Status = Status();
> >> >
> >> >will make the program useless while:
> >> >
> >> >Item_Status = Queue_Status();
> >> >
> >> >will leave the program useable. I think this is more a question on the
> >> >design of MFC than it is of a bug. Adding another function call to the stack
> >> >should not produce the result I am getting.
> >> >BRM
> >> Joseph M. Newcomer [MVP]
> >> email: newcomer@flounder.com
> >> Web: http://www.flounder.com
> >> MVP Tips: http://www.flounder.com/mvp_tips.htm
> >>
>
> Joseph M. Newcomer [MVP]
> email: newcomer@flounder.com
> Web: http://www.flounder.com
> MVP Tips: http://www.flounder.com/mvp_tips.htm
>