Re: [PATCH] Fix JavaHL error in SVNBase::createCppBoundObject

While reviewing r1882115, I wondered what was the original rationale
for the C++ function SVNBase::createCppBoundObject() to cache clazz's
ctor's method ID in a locally scoped static variable.

(In C++ such a variable is basically the same as a global variable,
only with its visibility limited to the scope of that function. So, as
Alexandr notes, the first call to that function sets the cache for all
later calls to the function, no matter the class on which it is
called, nor the clazz for which it is called.)

I don't know whether the GetMethodID call is time-consuming but I
hypothesized the caching might have been added for performance
reasons. So I did some digging and found that, on the contrary, the
function was implemented this way from the start.

SVNBase::createCppBoundObject() was originally added on the javahl-ra
branch in r1352400 and the caching was present in that original
version. Caching wasn't added later (to fix performance problems or a
bug, etc) and therefore there is nothing I can glean from the log
about any specific reason why it should be there. The mail thread
where the function was originally proposed as a patch [1] made no
mention of it either.

In any event, I am pretty sure the method ID should not be cached, and
definitely not in a locally scoped static variable. So, the change in
r1882115 looks good to me.

Cheers,
Nathan

[1] https://lists.apache.org/thread.html/7cda4e528f72ecd4793106822d544f45ac586d279f86bfa3deacc418%401339034634%40%3Cdev.subversion.apache.org%3E
Received on 2020-10-05 05:18:02 CEST