[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help)

From: Stefan Küng <tortoisesvn_at_gmail.com>
Date: 2007-07-01 17:52:52 CEST

Ivan Zhakov wrote:
> On 7/1/07, steveking <tortoisesvn@gmail.com> wrote:
>> Ivan Zhakov wrote:
>>
>> > Sorry, I was busy and I have not time to report my research results.
>> >
>> > My patch almost complete and I need somebody's eyes to look and
>> > confirm that I'm on right way (or say that I'm on wrong way :)
>> >
>> > I used Windows MIME database to convert page name to Windows page
>> > identifier.
>> > And most important question is it acceptable to read Windows MIME
>> > database directly from registry instead of using MLang API [1]? I
>> > didn't use MLang API, because it requires
>> > CoInitialize()/CoUninitialize().
>>
>> I think it would be better to use the API instead of reading the
>> registry. Because the registry is not documented and could change any
>> time.
>> About CoInitialize(): we could call that in svn_utf_initialize() and
>> call CoUninitialize in an apr cleanup handler.
> Actually registry is documented, at least for Windows CE :)
> I agree that using API is better, but it's not too easy. We have to
> call CoInitalize() in each thread that using COM. Another problem that
> we don't require user libraries to call svn_utf8_initalize().
>
> As workaround of problems above we can call to
> CoInitialize()/CoUninitalize() every time in win32_open_xlate(). This
> function get called only once for each page identifiers, so it
> shouldn't cause performance degradation. What do you think about it?

I forgot about the backward compatibility. Otherwise we could just
document that CoInitialize() must be called for every thread.

If the registry is documented, then we can of course use that. I don't
think it will be faster than using the API since registry accesses are
expensive too, but at least for most conversions it's not needed anyway.

[snip]
>> I know this is the correct way to do this, but I don't like it. You
>> allocate memory here which is needed only for a few lines of code. That
>> memory is allocated in the apr pool, which we don't know when it gets
>> cleared again (maybe not until the app exits because it's the same pool
>> that holds the translated strings). That means that a lot of memory is
>> 'wasted'.
>> For example, assume you do an
>> svn log -v url/to/kde/repository/root
>> and you'd have to keep that log information alive until your app exits.
>> With using an apr pool to reserve memory here that would mean that a
>> *lot* of memory is kept allocated but not used anymore.
>> That's why I used calloc() and free() in my patch - which isn't really
>> good either in terms of platform independence, but it won't consume a
>> lot of memory for nothing. Question is: do we really need apr pools here
>> for this Windows-specific code?
>> The correct way here may be to create a subpool and destroy that subpool
>> before the function exits. But that's an expensive operation.
>>
>> In case you're wondering why someone would do such a thing (get the log
>> info for a whole repository and keep it until the app exits), the
>> revision graph in TSVN needs to do exactly that.
> Yes, I know about this problem. I propose to discuss it separately
> after committing simplest implementation. Because we can made several
> optimizations: for example try to allocate small buffers on stack.

Yes, that would improve the performance a lot. Since most conversions
are done on paths, I suggest to allocate buffers smaller than
MAX_PATH*sizeof(WCHAR) on the stack, the rest could use an apr subpool.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Jul 1 17:52:58 2007

This is an archived mail posted to the Subversion Dev mailing list.