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

David M. Lloyd david.lloyd at redhat.com
Mon Mar 2 15:25:42 UTC 2009

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 (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)).

More information about the core-libs-dev mailing list