From: Dom Lachowicz (doml@appligent.com)
Date: Sat Aug 03 2002 - 23:01:58 EDT
Martin,
Sorry to say, but this argument is BS. We already do have poor-man's threads implemented via timers and idles, and all without the protection that things like mutexes and conditionals provide. I'll be impressed if anyone can tell me any singificant difference between having a thread that runs once every 1/2 second and a timer which fires every 1/2 second. In both cases you're running asynchronous bits of code which has the possibility to wreak havoc in our codebase. The major difference is that Pat's and my implementation actually protected certain bits of important data with a mutex. Sure, you do have some *non-atomic* booleans in there, but well, they're not atomic and not used consistently enough to protect all of our vital innards. The only real consideration I see here might be how OS drawing primitives play with threads, but even those can be easily circumvented since all we're doing is an XOR or Drawline call.
Further, these threads aren't poking around the text/ or fmt/ directories. All they do is manage blinking events. They don't touch the fmt or text directories. The code in the fmt directory simply tells a *non-threaded* object to queue a blink request. That's all. The threaded worker class simply loops waiting to service a blink request and goes back to sleep. To do this, it tells the view to draw or erase the insertion point. We're doing this from within a timer context anyway already.
The current cursor stuff is downright nasty. There are hundreds of mismatched enable and disable calls to the cursor and clearly something has to be done about it. It's responsible for lots of inconsistencies in the code (mostly regarding text selections) and cursor dirt. Our only salvation right now is the fact that there are more disable calls than enable calls, so dirt goes away via the benefit of whiting-out an already whited-out region. Having an object whose sole responsibility is to manage things like blinking is a good idea. Whether it gets implemented via a thread or a timer isn't a valid argument as we could easily have it implemented in terms of either with equivalent beneficial and detrimental results.
Sorry,
Dom
On Saturday, August 3, 2002, at 10:50 PM, Martin Sevior wrote:
> On Sun, 2002-08-04 at 06:43, Patrick Lam wrote:
>> On Sat, Aug 03, 2002 at 11:46:29AM +0100, Tomas Frydrych wrote:
>>>
>>> Martins doubts about my suggestion to move the cursor timer into a
>>> separate thread made me go back and study the spell checking
>>> problem bit more, and it turned out that the problem was not with the
>>> timer per se being blocked, but elsewhere in
>>> fl_BlockLayout::_doCheckWord, which I have now fixed.
>>>
>>> Enjoy orderly cursor blinking.
>>
>> That's cool. But I still think the cursor code really sucks, because
>> it's really ad-hoc and not abstracted. I think it should be fixed in
>> a more general way with threads.
>>
>
> I'm strongly not in favour of threads unless it can directly
> demonstrated that only threads can solve the problem at hand and even
> then I'd be worried. I'm by no means convinced that threads will help
> the cursor blinking problem, even with the nicely abstracted code from
> Mike's cursor class. Looking at this code it is clear that this really
> affects a lot of the code. I'd be really wary of implementing it unless
> the person doing the job was fully committed to following through on it
> and had no other major job with AbiWord.
>
> IMHO we'd be much better off fixing revision marks, hidden text,
> Footnotes, tables and import/exporting thereof etc before tackling this.
>
> Pat, you know how hard it is debug abi code, imagine if you had to deal
> with threads within the text/* directories.
>
> I think you could make a case for threads for the has downloaded as abi
> is blocked until the dictionary is downloaded. In addition it a rather
> isolated piece of code that does not affect the rest of the application.
>
> Cheers
>
> Martin
>
>
This archive was generated by hypermail 2.1.4 : Sat Aug 03 2002 - 23:09:43 EDT