Re: Threading problem in IconOverlay code

Tomas Kopal wrote:
> On 28.6.2007 12:23, Jan Rysavy wrote:
>> Hi Stefan and Tomas,
>>
>>> > - Change the global variables to be member variables of the COM
>> object,
>>> > thus making them per-instance.
>>>
>>> This one is clearly to be prefered. g_filepath, for instance, is
>>> only used in IsMemberOf (I simply told VS to grep all files). Thus,
>>> it can safely be made a member variable. My impression is that this
>>> is true for most globals.
>> I think that if you store g_filepath to each COM object, it will
>> lose its "caching" sense (other COM objects called for the same
>> filename need to use it).
>>
>> So I think that storing g_filepath (and others) to TLS is the best
>> solution to this problem. Or you can leave it global, but you must
>> access to these variables only in critical section.
> Hmm, after discussing this a bit further, I have to agree that TLS is
> probably the best solution here, at least for g_filepath. I will try to
> describe it a bit more how I understand it:
>
> Every icon kind has it's own handler, i.e. it's own COM registered in
> the registry. In TSVN case, which has about eight different icons
> depending on the file status, all eight handlers in registry points to
> the same COM object, which returns that the icon belongs to it based on
> file status.
> That means, that for a single file, there will be about eight calls in
> succession. That's what the g_filepath and other cache global variables
> are there most probably for - to avoid doing the same processing eight
> times in a row.
> All these calls will be done from one thread (remember, it's about
> finding one icon for one file). But all these calls will be on different
> instances of the COM interface (each corresponding to a different file
> state/icon).
>
> So, if we change the globals to instance member variables, we loose all
> the caching potential here. Leaving them global and protecting with e.g.
> critical section is better in case of access from one thread, but worse
> if multiple threads access TSVN in the same time.
>
> TLS means that the variable will be one per thread, i.e. the caching
> will work correctly for every thread without being disturbed by other
> threads running in parallel.
>
> So, what do you think now?

You're right. The reason for those global variables is to reduce disk
access. Because of the many overlay handlers (one for each overlay
icon), those global objects really improve the performance.
One problem however with TLS is that we don't have all 'simple' data
types but also complex objects.
I think the best way to go here is to use critical sections to guard the
global objects.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tortoisesvn.tigris.org
For additional commands, e-mail: dev-help@tortoisesvn.tigris.org