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?
>
>
> [snip]
> +static apr_status_t
> +win32_convert_to_stringbuf(win32_xlate_t *handle, const char *src_data,
> + apr_size_t src_length, svn_stringbuf_t **dest,
> + apr_pool_t *pool)
> +{
> + WCHAR * wide_str;
> + int retval, wide_size;
> +
> + *dest = svn_stringbuf_create("", pool);
> +
> + if (src_length == 0)
> + return APR_SUCCESS;
> +
> + retval = MultiByteToWideChar(handle->from_page_id, 0, src_data,
> src_length,
> + NULL, 0);
> + if (retval == 0)
> + return APR_FROM_OS_ERROR(GetLastError());
> +
> + wide_size = retval;
> + wide_str = apr_palloc(pool, wide_size * sizeof(WCHAR));
>
> 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.
--
Ivan Zhakov
---------------------------------------------------------------------
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:32:01 2007