[PATCH 0/2] Class- and class loader-local storage (Bug ID #6493635)

[David M. Lloyd]

On 02/27/2009 07:08 PM, Bob Lee wrote:
> David,
> 
> I regret making my suggestion in the first place. I really think we need 
> ephemerons, but for the sake of discussion:

OK, so you say that you don't think this feature should exist, period, 
until and unless ephemerons are implemented.  Why?  There's a need for it. 
  It does no harm.  Once ephemerons are implemented (if they ever are), my 
solution can *easily* be ported to use them without any API changes from 
the user perspective.  The difference is that I have a real solution, now, 
whereas ephemerons are vaporware, and will remain so for some unspecified 
amount of time.

> - Your patch adds 2 new classes. My suggestion adds one method (maybe 2 
> for convenience).

Mmm, okay.  I don't see this as an advantage or a disadvantage, just as a 
difference.

> - Your approach enables explicit clearing, but I thought the whole point 
> of adding this extension was to avoid explicit clearing. If you're going 
> to explicitly clear, why do you need this functionality at all? If we 
> want to support circular dependencies between class loaders, we should 
> pursue ephemerons because your solution requires explicit clearing 
> whereas ephemerons would not.

My solution supports implicit *and* explicit clearing (and so could/should 
your solution).  As long as you have a strong reference to the key, the 
data will remain.  If you lose the reference to the key, then the 
associated keys on all the maps become only weakly reachable.  Alternately 
you can explicitly clear the reference, which allows one to keep the 
Class*Local in a static without fear of introducing leaks (if that is how 
one wishes to use it).

> - Say for example that I need a static map from Class to List<Method> 
> (some filtered list of methods in Class). Your patch requires one 
> WeakHashMap per Class. My suggestion requires only one map total and one 
> lightweight data structure per ClassLoader.

Right.  The advantage to many hashmaps is a sort of implicitly striped 
access.  No hashmap would exist until data is initially associated with a 
class.  As I said in an earlier mail, disadvantage to a single, shared 
structure will ultimately be contention.  And of course the hashmap is 
simply an implementation detail - any specifically identified problems with 
it can be overcome using an alternate implementation (have any ideas?).

> - Your patch forces users to use your data structure. My suggestion 
> enables users to use whatever data structure they like. Your patch 
> introduces a point of contention between completely orthogonal 
> libraries. Mine introduces almost none (assuming you implement the 
> internal data structure in a non-locking fashion).

Now this is untrue.  My patch allows users to associate a piece of data 
(any object really) with a class.  Functionally there's little difference.

With my code:

    public final ClassLocal<Foo> fooHolder;

    ...in a method...
    fooHolder.set(someClass, someData);
    ...manually clear reference...
    fooHolder.clear();

With your suggestion (assuming you return a reference):

    public final Set<Reference<?>> refHolders;

    ...in a method...
    refHolders.add(someClass.getClassLoader().addReferenceTo(someData));
    ...manually clear references...
    refHolders.clear(); // assuming that's what this would do

In fact, I think my solution is far more intuitive to the end-user.  It's a 
nicer API, certainly more in line with what is in the JDK today (think 
ThreadLocals, *References, etc.).  And there need not be one single point 
of contention; it's more of a striped affair as long as you are storing 
data on classes (the only reason one exists is due to the naive 
implementation I chose - one could use a better hashmap (there is at least 
one lock-free implementation that I know of)).

- DML