On Tue, Nov 15, 2005 at 07:40:09AM +0100, Patrick Mayweg wrote:
> SVNClientSynchronized synchronizes on its class object, which can only
> exists once per class loader. It should be changed to the SVNClient
> class object, because that can only exists once per JVM. If you keep
> both classes in one Jar, that is not a real problem. But that is nothing
> new. On the other hand, using synchronization prevents only other
> threads to access something. It does not prevent reentracy.
>
Ok, that change makes sense, though I still don't understand why we need
such strict synchronisation. At the moment, SVNClientSynchronized is
acting as a barrier that allows only one call to be active per JVM,
rather than one per SVNClient object (as would be the case if the
SVNClient object wasn't thread-safe itself).
> I wrote SVNClientSynchronized in the first place, because I did see any
> statement about the subversion core libraries reentrant. So if any
> problems arise, it should be easy to switch a thread safe version.
The requirements for re-entrancy are pretty much a subset of the
requirements for thread safety, and with callbacks, re-entrancy problems
would show up even on a single thread. The Subversion core libraries
can be used in multi-threaded environments in both server configurations,
I believe.
> I will change JavaHL, so that JNIThreadData will no longer be stored in
> thread local storage but passed as parameter in almost all methods and
> put it into the batons. So that every call into the native library will
> keep its JNIEnv, no matter how the JVM schedules the java/native thread.
> But this is major change. I will not have time to do that for 1.3 and I
> do not think it should go 1.3 anyway.
>
I'm not sure that would be an improvement: you'd just be changing the
current explicit thread-local stack for one managed through the thread's
(thread-local) function-call stack, and the latter would have a lot
more duplication.
Then again, passing the JNIEnv to each function is easier to understand,
and it does remove a dependency on the APR TLS APIs. And I guess that
it also reduces the pressure on the (limited number of) TLS slots (and
the JNI spec explicitly mentions that JVMs can use thread-local storage
for the JNIEnv data, so we should expect that some will be in use).
But regardless of whether it's worthwhile in itself, you're right that
a change of that magnitude (even if it _is_ mostly just repetitive)
shouldn't go into 1.3, unless we've got an indication that it actually
fixes something (and at the moment, I can't see that it will have any
real effect at all).
Regards,
Malcolm
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Nov 15 11:11:54 2005