At 17:19 31/10/2004 +0100, you wrote:
>What do you mean by "just" the DevPartner profiler? I think that one is
>good enough, especially since it's free ;)
Well, it's not quite VTune!
>Not really ;)
>
>Fixed in revision 1884.
Great!
This is a summary of what I think I've found:
1. The registry caching mechanism doesn't work for reg keys which are not
found. This was initially noticeable with IsFixed, which because it was
relying on the default value rather than an actual set key, fell through
the RegQueryValueEx failed clause in CRegStdWORD::read, which doesn't set
m_read. Given that the caching period on the registry items is very
short, I would suggest that we set m_read = TRUE unconditionally in
CRegStdWORD::read. Once someone has changed some settings, these items
will get re-read after the registry cache timeout anyway.
2. The
if (sPath.compare(filepath)==0)
{
status = filestatus;
}
else
{}
bit in CShellExt::IsMemberOf doesn't work, because sPath is a native '\'
path, whereas filepath is a SVN (/) path. This is probably a good thing,
because there's no timeout mechanism on this 'cache'. I think we should
scrap this caching mechanism and rely on the 'proper' cache.
3. g_ShellCache.IsPathAllowed(sPath.c_str()) is run twice for icon overlays
- once in CShellExt::IsMemberOf and then again in GetFullStatus. This is
potentially quite an expensive function, with exclusion lists, inclusion
lists, etc. I added a parameter to GetFullStatus to say 'IsPathAllowed has
already been called' - you can't just remove IsPathAllowed from
GetFullStatus, because the column handler doesn't run it itself.
4. There is lots of duplicate checking of 'PathIsDirectory'. This is
checked in IsMemberOf, GetFullStatus and, for a cache miss, in
BuildCache. I changed GetFullStatus to take an 'isFolder' parameter, and
then propagated this down through the calls.
5. Related to '4', calls like PathIsDirectory and PathFileExists are
actually quite expensive, translating into calls right down into the
file-system. I think it might be advantageous in GetFullStatus to check
the cache *before* checking for the existence .SVN directory and the
EXCLUDEFILENAME file. I don't have good numeric data to prove this, but I
suspect you can do a fair bit of 'O(log(n))' lookup in a std::map for the
price of a call down to the file-system.
6. The CRT chdir call in PreserveChdir is expensive - much more so than
SetCurrentDirectory, because it sets up an environment variable for the
benefit of the CRT. I'm not convinced that's necessary on something which
is just preserving the directory for the safety of Explorer. Even if we
decided not to risk a change away from the CRT calls, it would be slightly
more efficient to use TCHAR and _tgetcwd/_tchdir, so that the Unicode build
used the proper API on Windows, and missed-out the OS's internal conversion
stage.
That's all I can think of for the moment... Let me know what dreadful
misapprehensions I've based it on!
Cheers,
Will
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tortoisesvn.tigris.org
For additional commands, e-mail: dev-help@tortoisesvn.tigris.org
Received on Sun Oct 31 19:23:35 2004