JBossDeveloper

By conversion, I meant converting a MethodCall <-> byte.

I also just realized that Method is used most often (which seems somewhat problematic in terms of performance). You could not replace java.lang.reflect.Method with a MethodCall (which is a method + arguments) where it might matter--like in Interceptors. But, one thing that you might be able to create if you had your own MethodCall class, is a canonicalization of Method objects... e.g. something akin to String.intern(). Then you'd able to write:

 if (meth == TreeCache.putDataEraseMethodLocal)) {

I'm not in favor of subclassing MethodCall, because that add a dependency on JGroups.
Why don't we simply compare the method IDs in the interceptors ?
In terms of prioritization, this doesn't seem to be very important as it gets a max gain of less than 1% as Elias mentioned, and this probably only in the perf tests...

I've working on this issue during the weekend and created a JIRA task for it:
http://jira.jboss.com/jira/browse/JBCACHE-581

Firstly, I separated the method declarations from TreeCache and methodid
definitions from EnhancedTreeCacheMarshaller onto a different class in order
to add clarity to the code.

I have also subclassed MethodCall in order to contain an id which is used in
comparisons in the code. I think it's a very neat way of doing it, but I agree
with Bela that it would create a dependency on JGroups.

The change itself affects a lot of the interceptors and other code. At the
moment, I'm trying to debug through some of the issues I had.

Subclassing MethodCall might not be as trivial as I thought it was.
Basically, whenever the constructor is called passing a Method instance,
there's no problem because I can use the Method to work out the id for
that method. However, there's cases in which MethodCall's empty
constructor is called(via EnhancedTreeCacheMarshaller). Looking at the
source code for MethoCall, there's two places in which method instance
variable is initialised, at the constructor and at readExternal.

For the latter case, where can I initialise the id? Override readExternal and
after calling super's readExternal, initialise id as per the content of
method?

I assume there was never a situation in TreeCache when the MethodCall's
method was compared and the method itself was null, am i correct?

Hope I explained myself clear :-)

why do we need to subclass MethodCall again ? I thought we created a hashmap consisting of IDs and MethodCalls which is populated at init time...

There are two objectives in this enhancement. First, improving the
performance of the Method comparison avoiding calls to equals() and
secondly, improving the readability of the code. Subclassing MethodCall is
more to do with the second one. Comparing method calls is not exactly the
same as comparing methods itself and it's proving a bit trickier than I
originally expected.

It is true that subclassing MethodCall creates a dependency on JGroups and it
could potentially mean that changes to internals on JGroups affected
JBossCache. How likely is this?

This is really my first time getting my hands dirty with JBossCache/JGroups,
so please, be gentle! :-)

No worries :-)

In general, MethodCall does not have an equals() method, so if you're doing the comparison in the interceptor, then that's fine.
I don't think adding an equals() to either MethodCall of a subclass would help, because it would look similar to the aforementioned impl in the interceptor... except for the case when we use IDs

We are talking about a JBoss Cache specific case here. As far as posisble, all method calls passed up the interceptor stack would be 'known' methods, i.e., methods which have been registered and assigned ids.

The prupose of subclassing MethodCall is so that the MethodCall contains - in addition to the Method and args - the id of this methd to improve the speed and readability of comparisons. E.g., instead of a series of if statements, a simple switch(call.getId()).

You're right in that there is no particular reason to subclass MethodCall itself - we could always write our own Method-container class thereby removing the dependency on JGroups, but this will also mean writing our own RpcDispatcher implementation (or again subclassing it).

The other problem with *not* subclassing MethodCall is that it breaks compatibility with custom Interceptors, thereby breaking an SPI. Subclassing MethodCall will not break this.

Why subclassing ? MethodCall.getId() will return the ID...

We're still on JGroups 2.2.8 which doesn't have this.

I'm happy to ship 1.4.0 with JGroups 2.2.9.1 so we have this functionality - then we just maintain the Method/id mappings in a table and change the .equals() comparisons in the interceptors to a switch statement.

Does anyone see any issues to switching to JGroups 2.2.9.1 for 1.4.0?

I think using JGroups 2.2.9.1 would make the change a lot simpler.

Alright guys, I'm gonna go ahead with the change which basically would be:

- Replacing Method.equals() for comparisons on the MethodCall.getId() (==)
- I'll pull out the Method declarations from TreeCache and the method id
declarations from EnhancedTreeCacheMarshaller and put them on another
class called MethodDeclarations. This should improve the readability.

Obviously, changing the dependency to JGroups 2.2.8 to 2.2.9.1 as well