commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Romain Manni-Bucau <>
Subject Re: [jcs] JCache implementation review
Date Fri, 02 May 2014 15:17:44 GMT

globally a big +1

some more remarks inline

2014-05-02 17:13 GMT+02:00 Thomas Vandahl <>:
> Hi folks,
> I took some time to look over the code of the JCache implementation and
> I have some suggestions for simplification (under the assumption that I
> understood the intention correctly).
> - JCS does not implement the same model of cache element expiry, however
> a few more existing features could be used. The question is how strict
> are the requirements of the JSR. I think that most of the functionality
> of JCSElement is already there in ICacheElement.

would be awesome to merge if you think it is possible. "strict" part
can be tested running TCKs (once the few missing assertNotNull and the
shutdown are in place - didnt check today). Only interceptor related
ones should fail. If ExpiryTest fails it is a regression.

> - I'd like to move Statistics to the core. The cache statistics of JCS
> are too scattered and this class would very much improve this situation.

Ok. Was expecting something to compute distributed statistics and not
only local ones. Do you think we can without breaking the whole

> - JCS has element event handling built-in. It works, however, for expiry
> and spool events only. I made a few modifications so that the additional
> events for create, update etc. can be added easily.

great news! If we can move listeners to just be internal handler
listener adapters it would be awesome.

> - The code produces quite a few warnings referring to type safety and
> possible NPEs. I guess this could be improved.

go for it when you see it :)

> - The code of the core and jcache modules compiles fine here and all
> tests pass (MacOS 10.6.8, JDK 1.6.0_65). The 1.6 compiler needed some
> additional support with CacheTest, however.
> - Wonder where the commons-math3 dependency comes from?

shouldn't be needed IMHO

> Bye, Thomas.
> ---------------------------------------------------------------------
> To unsubscribe, e-mail:
> For additional commands, e-mail:

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message