Login | Register
My pages Projects Community openCollabNet

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

subversion
Discussion topic

Back to topic list

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

Author steveking
Full name Stefan Küng
Date 2007-07-01 08:52:52 PDT
Message Ivan Zhakov wrote:
> On 7/1/07, steveking <tortoisesvn at gmail dot 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()/CoUni​nitialize().
>>
>> 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

« Previous message in topic | 19 of 35 | Next message in topic »

Messages

Show all messages in topic

[PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) chemodax deleted 2007-06-30 14:45:41 PDT
     Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) djh D.J. Heap 2007-06-30 19:18:41 PDT
         Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) djh D.J. Heap 2007-06-30 19:43:51 PDT
             Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) chemodax deleted 2007-06-30 23:58:44 PDT
                 Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) djh D.J. Heap 2007-07-01 06:06:02 PDT
                     Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) Daniel Rall <dlr at collab dot net> Daniel Rall <dlr at collab dot net> 2007-07-03 09:58:00 PDT
                         Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) djh D.J. Heap 2007-07-04 07:48:13 PDT
             Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) chemodax deleted 2007-07-05 12:48:46 PDT
                 Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) djh D.J. Heap 2007-07-05 13:21:09 PDT
                     Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) Mark Phippard <markphip at gmail dot com> Mark Phippard <markphip at gmail dot com> 2007-07-05 13:23:59 PDT
                 Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) djh D.J. Heap 2007-07-07 08:31:40 PDT
                     Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) chemodax deleted 2007-07-07 15:32:02 PDT
                         Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) chemodax deleted 2007-07-07 15:47:49 PDT
                             Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) djh D.J. Heap 2007-07-07 18:47:34 PDT
                                 Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) chemodax deleted 2007-07-08 01:06:47 PDT
                         Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) djh D.J. Heap 2007-07-07 18:32:05 PDT
     Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) steveking Stefan Küng 2007-07-01 00:13:50 PDT
         Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) chemodax deleted 2007-07-01 08:31:56 PDT
             Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) steveking Stefan Küng 2007-07-01 08:52:52 PDT
                 Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) chemodax deleted 2007-07-02 04:07:15 PDT
                     Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) djh D.J. Heap 2007-07-02 20:49:17 PDT
                         Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) chemodax deleted 2007-07-05 02:11:42 PDT
                             Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) Mark Phippard <markphip at gmail dot com> Mark Phippard <markphip at gmail dot com> 2007-07-05 05:58:48 PDT
                                 Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) chemodax deleted 2007-07-05 06:07:05 PDT
                                 Re: [PATCH] Remove APR ICONV dependency on Windows (was SVN Win32 Developers -- need some help) steveking Stefan Küng 2007-07-05 06:10:53 PDT
Page: of 2 « Previous | Next »
Messages per page: