Re: Explorer hanging - 1.6.1

Stefan Fuhrmann wrote:

>>> * Showing the context menu or opening a folder
>>> in the Explorer takes 1..2 additional seconds.
>>> This is fine with me as long as it remains in
>>> that order of magnitude.
>> That's because TSVN asks for the status of the right-clicked item before
>> showing the menu.
>
> Oops, I forgot to add that to the bullet point:
> I see the same delay when opening a folder.

That's not good. At least not when opening the folder for a second time.
The first time is a little bit different since then the cache doesn't
have anything about that folder yet.

>>> From what I have seen of the TSVNCache access code,
>>> there are possible explanations for both effects:
>>>
>>> * It seems that there is a sequential Explorer->TSVNCache
>>> round-trip for every item in the explorer. If that
>>> observation is correct, unfortunate scheduling could
>>> cost one or even two time-slices (~30 ms) per item.
>>>
>>> -> maybe, we could send the state for *all* nodes in
>>> the same folder as answer, if available. The explorer
>>> plugin could then cache the info for a few msecs.
>> Explorer doesn't ask for the overlays in an ordered fashion. And not
>> even all from the same folder - depending on whether the tree view is
>> shown or not and also depending on OS, it also asks for the parent
>> folder or even the folder above that.
>
> My idea was returning more information than what was
> requested is virtually free as long as that information
> is already available in TSVNCache.
>
> I don't assume any specific request order: if one of
> the following requests 'accidentally' matches the info
> cached locally in the Explorer plugin, no TSVNCache
> request is necessary. Such local cache should keep,
> say, the last 10 results and use them if they are not
> older than about 200 msecs.

This might work. But I wouldn't bet on it.

>>> * There is a 10 second time-out for TSVNCache to answer
>>> a request. So, the Explorer might frequently ask for
>>> unavailable drives / folders, for some reason.
>>>
>>> -> maybe, we can come up with an asynchronous protocol
>>> between TSVNCache and the Explorer. There must
>>> already be something like that in place for
>>> nested folder status updates. So, there would be
>>> no reason to wait for data that has not already been
>>> fetched by TSVNCache.
>> The problem is not so much the status of items which are not yet in the
>> cache. It's that if a status request from the explorer should be
>> answered but the cache itself is locked because it is currently fetching
>> the status of an item (could be the same or another), the cache can't
>> answer that request (it's locked) until the call to svn_client_status()
>> returns.
>
> So, we need to rework the cache update code
> (receive data to temp struct -> lock -> update
> from temp -> unlock).
>
> I just so happen to know someone with extensive
> experience in multi-threading and cache structures ;)

Good luck with that!
Seriously: even if you know a lot about multi-threading and cache
structures, you will get into troubles.
For example, there's a possible deadlock over process boundaries: if the
cache calls an API that (even if not documented!) somehow sends a
notification to the shell that something has changed (yes, a lot of file
API's do that!), the shell asks for the status again and calls into the
cache (see the deadlock here?).

And the problem I just found: RegisterDeviceNotification() sends a
message to the UI thread, but our UI thread is waiting for the lock of
the directory watcher, but the call to RegisterDeviceNotification() has
that lock already --> deadlock.

>> There's a two second timeout: it the cache can't get a lock to answer an
>> explorer request within two seconds, it returns 'unversioned' to not
>> block that request for too long.
>>
>>
>>> Now, I tested Wednesday's nightly (r16319) and this
>>> is what I found:
>>>
>>> * Speed is back to normal as it was during 1.6.0
>>> development, maybe slightly better than 1.5.x.
>>> The above mentioned effects are still there.
>>>
>>> * There seems to be a serious flaw that should not
>>> be released with 1.6.2. No overlays are shown
>>> within the top-level folder of a large WC. Everything
>>> works fine for the top-level folder (e.g. /trunk)
>>> itself and for smaller projects (<=10k files)
>>> as well. F5, reboot, un-install TSVN etc. do not
>>> fix it.
>> But this (I think) was also the case in earlier versions. If you have so
>> many files in one folder, explorer stops asking for status after a
>> certain time.
>
> What I meant is that the folder sub-tree must be large.
> The top-level folder itself only contains about 10
> sub-folders. My guess is that it takes too long to
> combine the recursive status info and the above
> mentioned 2 second time-out suddenly cancels the
> request.

Hmmm - could you try and debug this? There might be a function that is
not very good (O()-wise) and could cause this.

>>> What do you think? Shall we put a thorough revision of
>>> TSVNCache on the 1.7 TODO-list?
>> Be my guest!
>> But you might have noticed that even small changes can have a big
> impact.
>
> You are absolutely right. The problems we are facing
> with TSVNCache are actually long-standing issues, though.
> There were always reports that it misbehaved one way
> or another.
>
> My gut feeling is that without addressing the conceptional
> weaknesses we will always end up with the 'whack-a-mole'
> style bug fixing that we suffered over the last weeks.

Another thing to remember: keeping the status 'good' recursively is
hard, especially with externals, links, hardlinks, mount points, ...
A *lot* of code in the cache had to be added because of *that*. And no
matter how good a concept you come up with, you still have to deal with
all those situations and problems.

Stefan

-- 
       ___
  oo  // \\      "De Chelonian Mobile"
 (_,\/ \_/ \     TortoiseSVN
   \ \_/_\_/>    The coolest Interface to (Sub)Version Control
   /_/   \_\     http://tortoisesvn.net
------------------------------------------------------
http://tortoisesvn.tigris.org/ds/viewMessage.do?dsForumId=757&dsMessageId=2115231
To unsubscribe from this discussion, e-mail: [dev-unsubscribe_at_tortoisesvn.tigris.org].