Looks good, my comments inline:
On 10 Apr 2009, at 12:43, Galder Zamarreno wrote:
Hi,
Re:
https://jira.jboss.org/jira/browse/ISPN-44
I've attached a patch to the JIRA containing a 1st implementation of
a JBoss Marshalling based marshalling layer for Infinispan. A few
notes:
- JBMAR based marshalling is optional for the moment. To enable it,
configure the marshaller to be
org.infinispan.marshall.jboss.JBossMarshaller.
- I created a new profile that runs the testsuite with this
marshaller. To run, just do: mvn -Ptest-jbossmarshaller test.
I am guessing this is just a temporary measure to compare the two
approaches to marshalling? In the end I don't want to ship with 2
different approaches to marshalling.
- I've run the testsuite for core and there're no regressions
with
new marshaller. I also wanted to test jdbc but the testsuite seems
to need a bit of tidying up or I need to preconfigure something
(Tests run: 177, Failures: 4, Errors: 0, Skipped: 164, Time elapsed:
0.631 sec <<< FAILURE!). Regardless, jdbc uses a dummy marshaller so
would be pointless for the moment.
Yes, ignore this, I believe Mircea is looking at this. And, as you
said, this uses a dummy marshaller so it should not affect you.
- Different type marshalling has been implemented using
org.jboss.marshalling.Externalizer implementations mapped from
HorizonMarshaller class. JBMAR does support Externalizable classes
but I figured it out later and wanted to touch as less of existing
code as possible. So, for Beta1, some of Externalizer
implementations (except JDK classes) will probably go in favour of
Externalizable implementations.
- StateTransferManagerImpl uses marshaller in a different way to the
rest. It first opens an ObjectOutput and then makes multiple calls
to stream stuff. StateTransferManagerImpl was hardcoded to use OOS
and OIS so I had to add org.infinispan.marshall.Marshaller that
would start/finish these OO/OI implementations. This is to enable
JBossMarshaller to provide OO/OI implementations coming from JBMAR.
- I had one doubt about the scope of the marshaller. I gave it
@Scope(Scopes.GLOBAL) since it'd be something shared among all
caches. Now, I assume its @Stop method would be called when all
caches are finished and the CacheManager is stopped? Wanted to make
sure nothing is leaked!.
Yes, your assumption is correct. The whole thing with scopes is that
2 separate component registries are created, one for global components
and one for each cache instance. And lifecycle (such as @Stop) is
controlled by the CR so even if you stop individual caches, as long as
the cache manager is running global components won't be stopped.
Global components are only stopped when the cache manager is stopped.
- Application level versioning (VAM) is not yet supported in JBMAR
but David is happy to listen for ideas. I'll address that for Beta1.
For now we could write our own version headers (as we do with the VAM).
- For Beta1, I'd like David to have a look at what I've done
and see
if he can review it and then do perf tests to see whether this new
implementation is faster than our current one. If it is, next step
would be to design a way for people to extend/modify via
configuration magic numbers for their classes, provide Externalizers
for their own classes...etc.
I know this is jumping the gun a little, but how does this play with
ISPN-42: Object stream pooling? That's an optimisation I would really
like to see in place as soon as possible. :-) Is this a trivial
"extra" on top of ISPN-44?
Cheers
--
Manik Surtani
manik(a)jboss.org
Lead, Infinispan
Lead, JBoss Cache
http://www.infinispan.org
http://www.jbosscache.org