Re: review request (L): 7030453: JSR 292 ClassValue.get method is too slow
* John Rose: But, in order to respect the general aim you are mentioning, I have unhoisted one of the two words from the Class instance itself. This will cause a minor slowdown in JSR 292 use cases. What about using ClassValue for the various caches instead? enumConstants and enumConstantDirectory seem good candidates (callers cache the value anyway or do additional work while accessing the field). -- Florian Weimerfwei...@bfk.de BFK edv-consulting GmbH http://www.bfk.de/ Kriegsstraße 100 tel: +49-721-96201-1 D-76133 Karlsruhe fax: +49-721-96201-99 ___ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
Re: review request (L): 7030453: JSR 292 ClassValue.get method is too slow
On Dec 8, 2011, at 1:55 AM, Florian Weimer wrote:
* John Rose:
But, in order to respect the general aim you are mentioning, I have
unhoisted one of the two words from the Class instance itself. This
will cause a minor slowdown in JSR 292 use cases.
What about using ClassValue for the various caches instead?
enumConstants and enumConstantDirectory seem good candidates (callers
cache the value anyway or do additional work while accessing the field).
That's a fine idea, Florian, especially when we are counting every word of
fixed overhead. (The alternative is keeping one root pointer in Class for the
whole block of caches.)
Even the reflective caches are candidates for ClassValue treatment, since only
a minority of classes are subjected to reflective operations.
ClassValue is good for any set of values associated sparsely with classes, as
long as the query does not happen in a very tight loop.
The trade-off is whether to add another 4-5 cache line touches per use to buy
the extra compactness. To compare queries:
Class cls = ...;
Object val1 = cls.cache1;
if (val1 != null) ... // fill cache
test1(val1);
load $t1, [$cls + #Class::cache1]
test $t1
jump,zero Lfillcache
call test1($t1)
ClassValue cval = ...
Object val2 = cval.get(cls);
test2(val2);
load $t1, [$cls + #Class::classValueMap]
load $t2array, [$t1 + #ClassValueMap::cacheArray]
implicit { test $t1; jump,zero Lfillcache } // via trap handler
load $t3, [$t2array + #Object[]::length]
sub $t3, 1
jump,negative Lfatal // never taken; software invariant
load $t4a, [$cval + #ClassValue::hashCodeForCache]
load $t4b, [$cval + #ClassValue::version]
and $t4a, $t3
load $t5entry, [$t2array + $t4a*wordSize + #Object[]::base]
load $t6a, [$t5entry + #Entry::value]
load $t6b, [$t5entry + #Entry::version]
cmp $t6b, $t5b
jump,notEqual Lfillcache
call test2($t6a)
The pointer dependencies for cache references are:
ClassValue - t4:{hashCodeForCache,version}
Class - t1:ClassValueMap - t2:cacheArray - ( t3:length, t5:Entry -
t6:{value,version} )
The bracketed items are likely to be on a single cache line, so there are six
cache references. For a constant ClassValue, the t4 references can (in
principle) be hoisted as constants into the code. And the first indirection
($t1) could be removed by hoisting the cache array back into Class.
All this reminds me...
Somebody should experiment with re-implementing reflection and proxy creation
on top of method handles. It would let us cut out a bunch of old code (both
C++ and Java), and standardize on a single high-leverage mechanism for runtime
method composition. (Look in the current JDK core where bytecode spinning is
happening... Those places are all candidates for refactoring with method
handles.)
We are working on tuning up method handle invocation performance (e.g.,
7023639). When method handles are robustly performant, we will have the
attractive option of refactoring older APIs on top of MHs.
It's not too early to start experimenting with a POC for this. It would be a
nice big open-source project. Any takers?
-- John___
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
Re: review request (L): 7030453: JSR 292 ClassValue.get method is too slow
On 12/04/2011 08:38 PM, Joe Darcy wrote: Hi John, Are there alternatives to adding two new fields to java.lang.Class? I assume most Class'es won't have ClassValue information associated with them. -Joe If you use Groovy, JRuby or Nashorn in your code, all visible classes will use this two fields. Any alternative will slow down the access to the class value. BTW, if we had to remove some fields, I vote for moving all fields related to the reflection in a delegate class. Looking up for members is slow so most of the code that call reflection methods use their own cache, so the performance impact will be small in my opinion (I have no data to prove that :) Rémi ___ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
Re: review request (L): 7030453: JSR 292 ClassValue.get method is too slow
On 12/05/2011 01:27 AM, Joe Darcy wrote: On 12/4/2011 2:13 PM, Rémi Forax wrote: On 12/04/2011 08:38 PM, Joe Darcy wrote: Hi John, Are there alternatives to adding two new fields to java.lang.Class? I assume most Class'es won't have ClassValue information associated with them. -Joe If you use Groovy, JRuby or Nashorn in your code, all visible classes will use this two fields. Any alternative will slow down the access to the class value. In the mean time, all the non-Groovy, non-JRuby, non-Nashorn, etc. uses of class Class and all the classes not visible in those environments when they are being used will be larger. Adding the fields may be the right time/space trade-off, but I think the point merits some discussion given how many Class objects get created and the relative proportion of Java executions where ClassValue is currently used. The more reasonable time/space trade-off can change over time of course. -Joe I agree but as I said, in that case, I think it's better to take a look to the big picture and see if not only class values fields but also annotations related fields or reflection related fields can be moved. Also, if we don't provide a fast ClassValue, people will create their own concurrent weak hash map using class as key that will be worst because it seems that only few people knows how to do that right. Personally, I don't. Rémi ___ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
Re: review request (L): 7030453: JSR 292 ClassValue.get method is too slow
The caching ClassValue is still under review. I took the opportunity to remove -Xlint warnings (adding type parameters) and refactor for better readability. Also, Mike Duigou pointed out that there was still a dependency on ClassValue.equals == Object.equals, which could create a side-channel between independent CVs. I removed this by replacing ClassValue with ClassValue.Identity as the hash table key. The cache access is racy, for speed, but all races are (thought to be) benign. For cache hits, end-user values are obtained from non-racing accesses. All cache state changes happen either under a per-class lock or during GC (nulling of weak references). Could I have a re-review? Thanks, -- John On Sep 20, 2011, at 4:10 PM, John Rose wrote: On Sep 19, 2011, at 2:58 PM, John Rose wrote: http://cr.openjdk.java.net/~jrose/7030453/webrev.00 After some comments from David Holmes (thanks David!) I added internal comments about race conditions. I refreshed the webrev with the additional comments. I also changed one variable to be volatile, with a paragraph of comments explaining why. (The change to volatile will inhibit CSE of ClassValue.get calls, but I think such CSE was unlikely anyway. There should be no other performance effects.) -- John ___ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
Re: review request (L): 7030453: JSR 292 ClassValue.get method is too slow
On Sep 19, 2011, at 11:58 PM, John Rose wrote: http://cr.openjdk.java.net/~jrose/7030453/webrev.00 src/share/classes/java/lang/ClassValue.java: 233 /** Value stream for for hashCodeForCache. See similar structure in ThreadLocal. */ Two for's. 578 * from the ehad of a non-null run, which would allow them ehad? Otherwise I think this looks good. -- Christian The existing JDK 7 implementation of ClassValue is a place-holder which is defective in several ways: - It uses cascaded WeakHashMaps to map from (Class, ClassValue) pairs to values, which is slow. - It does not lock the root WeakHashMap, which can cause a race condition the first time a class is encountered. - It relies on internal details of WeakHashMap to avoid other race conditions. The new implementation uses a concurrently readable cache per Class object with entry versioning to manage lookups. It is more correct and scalable. The tunable parameters CACHE_LOAD_LIMIT and PROBE_LIMIT were chosen by looking at the behavior of artificial workloads. Experience with real workloads will probably lead to further modifications (under new Change Requests). I thought of making them tunable from JVM command line properties, but since no other class in java.lang does this, I held back. The previous implementation had a store barrier which pushed (via lazySet) pending store values from the user-supplied value before the ClassValue mapping was published. I removed it because it is a false fix for user-caused race conditions. (False because it has the desired effect only on some platforms.) I think it is better to put that issue back onto the user. We still need a memory fence API to give users the right hook for such problems. There is a package-private change to java.lang.Class, adding two new fields (to the existing 19 fields declared in Class.java). Although this class is in java.lang, it is part of JSR 292. Therefore the review comments will be collected in mlvm-dev. The review request is CC-ed to hotspot-compiler (where JSR 292 changes are pushed) and core-libs (which is responsible for java.lang). -- John ___ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
Re: review request (L): 7030453: JSR 292 ClassValue.get method is too slow
On Sep 20, 2011, at 1:02 AM, Christian Thalinger wrote: On Sep 19, 2011, at 11:58 PM, John Rose wrote: http://cr.openjdk.java.net/~jrose/7030453/webrev.00 src/share/classes/java/lang/ClassValue.java: 233 /** Value stream for for hashCodeForCache. See similar structure in ThreadLocal. */ Two for's. 578 * from the ehad of a non-null run, which would allow them ehad? head Otherwise I think this looks good. Thanks! -- John ___ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
Re: review request (L): 7030453: JSR 292 ClassValue.get method is too slow
On Sep 19, 2011, at 2:58 PM, John Rose wrote: http://cr.openjdk.java.net/~jrose/7030453/webrev.00 After some comments from David Holmes (thanks David!) I added internal comments about race conditions. I refreshed the webrev with the additional comments. I also changed one variable to be volatile, with a paragraph of comments explaining why. (The change to volatile will inhibit CSE of ClassValue.get calls, but I think such CSE was unlikely anyway. There should be no other performance effects.) -- John ___ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
review request (L): 7030453: JSR 292 ClassValue.get method is too slow
http://cr.openjdk.java.net/~jrose/7030453/webrev.00 The existing JDK 7 implementation of ClassValue is a place-holder which is defective in several ways: - It uses cascaded WeakHashMaps to map from (Class, ClassValue) pairs to values, which is slow. - It does not lock the root WeakHashMap, which can cause a race condition the first time a class is encountered. - It relies on internal details of WeakHashMap to avoid other race conditions. The new implementation uses a concurrently readable cache per Class object with entry versioning to manage lookups. It is more correct and scalable. The tunable parameters CACHE_LOAD_LIMIT and PROBE_LIMIT were chosen by looking at the behavior of artificial workloads. Experience with real workloads will probably lead to further modifications (under new Change Requests). I thought of making them tunable from JVM command line properties, but since no other class in java.lang does this, I held back. The previous implementation had a store barrier which pushed (via lazySet) pending store values from the user-supplied value before the ClassValue mapping was published. I removed it because it is a false fix for user-caused race conditions. (False because it has the desired effect only on some platforms.) I think it is better to put that issue back onto the user. We still need a memory fence API to give users the right hook for such problems. There is a package-private change to java.lang.Class, adding two new fields (to the existing 19 fields declared in Class.java). Although this class is in java.lang, it is part of JSR 292. Therefore the review comments will be collected in mlvm-dev. The review request is CC-ed to hotspot-compiler (where JSR 292 changes are pushed) and core-libs (which is responsible for java.lang). -- John ___ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
Re: review request (L): 7030453: JSR 292 ClassValue.get method is too slow
Hi John, On Tue, Sep 20, 2011 at 5:58 AM, John Rose john.r.r...@oracle.com wrote: The tunable parameters CACHE_LOAD_LIMIT and PROBE_LIMIT were chosen by looking at the behavior of artificial workloads. Experience with real workloads will probably lead to further modifications (under new Change Requests). I thought of making them tunable from JVM command line properties, but since no other class in java.lang does this, I held back. FYI, There's one, java.lang.Integer's cache size is tunable via JVM command line flag -XX:AutoBoxCacheMax, which in turn sets Java system property java.lang.Integer.IntegerCache.high, and affects the Integer cache size. But that's probably a special case anyway. Regards, Kris Mok ___ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
Auto Reply: Re: review request (L): 7030453: JSR 292 ClassValue.get method is too slow
On vacation till October 3. Will be reading email occasionally. ___ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
Re: review request (L): 7030453: JSR 292 ClassValue.get method is too slow
On Sep 19, 2011, at 7:00 PM, Krystal Mok wrote: FYI, There's one, java.lang.Integer's cache size is tunable via JVM command line flag -XX:AutoBoxCacheMax, which in turn sets Java system property java.lang.Integer.IntegerCache.high, and affects the Integer cache size. But that's probably a special case anyway. Thanks for the reminder, Kris. -- John___ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
