Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-10 Thread Chris Hegarty
On 07/12/2012 15:46, Remi Forax wrote: On 12/07/2012 04:34 PM, Chris Hegarty wrote: I filed 8004707: Remove superfluous access$000 methods in java.util, to track this issue. I can file a separate issue for java.lang. yes, please do that. I filed 8004797: Remove superfluous access$000

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-10 Thread Remi Forax
On 12/10/2012 11:51 AM, Chris Hegarty wrote: On 07/12/2012 15:46, Remi Forax wrote: On 12/07/2012 04:34 PM, Chris Hegarty wrote: I filed 8004707: Remove superfluous access$000 methods in java.util, to track this issue. I can file a separate issue for java.lang. yes, please do that. I

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-07 Thread Doug Lea
On 12/06/12 18:44, Remi Forax wrote: BTW, at some point, it will be a good idea to get ride of all these method access$000 in java.lang/java.util at least. Maybe this is out of scope for this CR, but while in the neighborhood of ThreadLocal, it could be done here. There are a couple of

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-07 Thread Chris Hegarty
I filed 8004707: Remove superfluous access$000 methods in java.util, to track this issue. I can file a separate issue for java.lang. I'm sure there are other ways, but a simple find reports them: :pwd build/solaris-i586/classes/java/util : find . -name *.class -exec javap -v {} \; | grep

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-07 Thread Remi Forax
On 12/07/2012 04:34 PM, Chris Hegarty wrote: I filed 8004707: Remove superfluous access$000 methods in java.util, to track this issue. I can file a separate issue for java.lang. yes, please do that. I'm sure there are other ways, but a simple find reports them: :pwd

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-06 Thread Remi Forax
On 12/06/2012 06:10 AM, David Holmes wrote: On 6/12/2012 5:39 AM, Akhil Arora wrote: This patch adds a constructor to ThreadLocal to supply initial values using the new Supplier functional interface. Please review. This work was done by Jim Gish.

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-06 Thread Doug Lea
On 12/06/12 03:23, Remi Forax wrote: On 12/06/2012 06:10 AM, David Holmes wrote: On 6/12/2012 5:39 AM, Akhil Arora wrote: This patch adds a constructor to ThreadLocal to supply initial values using the new Supplier functional interface. Please review. This work was done by Jim Gish.

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-06 Thread Vitaly Davidovich
Doug, When you see the fast to slow ThreadLocal transition due to class loading invalidating inlined get(), do you not then see it get restored back to fast mode since the receiver type in your call sites is still the monomorphic ThreadLocal (and not the unrelated subclasses)? Just trying to

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-06 Thread Vitaly Davidovich
Understood - I'm just trying to make sure I don't have the wrong mental model of this in my mind. Taking pathology aside (e.g. code cache is full when time to recompile, poor tiering interaction, etc), I'd expect the fast inlined path to be restored assuming call site type profile (of the ones we

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-06 Thread Akhil Arora
On 12/06/2012 03:32 AM, Doug Lea wrote: These seem like really good reasons to implement as you suggested in last post, with a static factory that uses a non-public *final* subclass that wires initialValue to supplier call, and not otherwise modifying the ThreadLocal class. It has the added

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-06 Thread Chris Hegarty
Thank you Akhil, this looks much better. I'm not completely comfortable with, The initial value of the variable is provided by calling the {@code intialValue} method. Similarly, The initial value of the variable is determined by invoking the {@code get} method on the {@code Supplier.

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-06 Thread Doug Lea
On 12/06/12 12:01, Akhil Arora wrote: redone - http://cr.openjdk.java.net/~akhil/8003246.1/webrev/ Much better; thanks! As a minor matter, it probably makes sense to give that class a name. Even though it is a non-public static class, the name will be seen here and there by debuggers,

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-06 Thread Akhil Arora
On 12/06/2012 09:38 AM, Doug Lea wrote: Much better; thanks! As a minor matter, it probably makes sense to give that class a name. Even though it is a non-public static class, the name will be seen here and there by debuggers, stacktraces, etc, and the default $ name will be confusing. As in:

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-06 Thread Akhil Arora
On 12/05/2012 09:10 PM, David Holmes wrote: Was there a CCC request for this? Yes, a CCC was submitted on Nov 12, but it was deferred pending a discussion on this alias. I just updated and resubmitted the CCC based on Doug and Remi's suggestions.

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-06 Thread Doug Lea
On 12/06/12 13:00, Akhil Arora wrote: http://cr.openjdk.java.net/~akhil/8003246.2/webrev/ - added SuppliedThreadLocal inner class - reverted javadoc changes to no-args constructor - added null check for Supplier Great! Thanks for the quick response. -Doug

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-06 Thread Peter Levart
There's a quick trick that guarantees in-lining of get/set/remove: public static class FastThreadLocalT extends ThreadLocalT { @Override public final T get() { return super.get(); } @Override public final void set(T value) { super.set(value); }

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-06 Thread Remi Forax
On 12/06/2012 07:29 PM, Doug Lea wrote: On 12/06/12 13:00, Akhil Arora wrote: http://cr.openjdk.java.net/~akhil/8003246.2/webrev/ - added SuppliedThreadLocal inner class - reverted javadoc changes to no-args constructor - added null check for Supplier Great! Thanks for the quick response.

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-06 Thread Remi Forax
On 12/06/2012 08:01 PM, Peter Levart wrote: There's a quick trick that guarantees in-lining of get/set/remove: public static class FastThreadLocalT extends ThreadLocalT { @Override public final T get() { return super.get(); } @Override public final void

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-06 Thread Remi Forax
On 12/06/2012 01:12 PM, Vitaly Davidovich wrote: Understood - I'm just trying to make sure I don't have the wrong mental model of this in my mind. Taking pathology aside (e.g. code cache is full when time to recompile, poor tiering interaction, etc), I'd expect the fast inlined path to be

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-06 Thread Akhil Arora
On 12/06/2012 11:02 AM, Remi Forax wrote: just nitpicking, return new SuppliedThreadLocalT(supplier); should be return new SuppliedThreadLocal(supplier); Also, can you add a @see in the constructor of ThreadLocal that references ThreadLocal.withInitialValue(Supplier).

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-06 Thread Alan Bateman
On 06/12/2012 19:43, Akhil Arora wrote: http://cr.openjdk.java.net/~akhil/8003246.3/webrev/ also incorporates a nitpick from Mike - Objects.requireNonNull returns its arg Thanks - nitpicking welcome :) Would it be worth including a note in the withInitial's javadoc to make it clear that

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-06 Thread Peter Levart
On 12/06/2012 08:08 PM, Remi Forax wrote: On 12/06/2012 08:01 PM, Peter Levart wrote: There's a quick trick that guarantees in-lining of get/set/remove: public static class FastThreadLocalT extends ThreadLocalT { @Override public final T get() { return super.get(); }

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-06 Thread Peter Levart
Ok, I've got an explanation. It's not because of using different static type of variables in code with final methods, but because TL0 was redirected to a separate method with separate call sites. The same happens using this variant: static class TL0 extends ThreadLocalInt {} static

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-06 Thread Remi Forax
On 12/06/2012 09:38 PM, Peter Levart wrote: On 12/06/2012 08:08 PM, Remi Forax wrote: On 12/06/2012 08:01 PM, Peter Levart wrote: There's a quick trick that guarantees in-lining of get/set/remove: public static class FastThreadLocalT extends ThreadLocalT { @Override public

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-06 Thread Jeff Hain
nitpicking welcome :) AFAIK (*), removing 'private' for supplier field could prevent javac to generate accessors for this field, in case it would be accessed from outside inner class (but since it's not, it would not yield better perfs, just smaller bytecode). (*) If I correctly undertood what

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-06 Thread Remi Forax
On 12/06/2012 11:05 PM, Jeff Hain wrote: nitpicking welcome :) AFAIK (*), removing 'private' for supplier field could prevent javac to generate accessors for this field, in case it would be accessed from outside inner class (but since it's not, it would not yield better perfs, just smaller

RFR: 8003246: Add Supplier to ThreadLocal

2012-12-05 Thread Akhil Arora
This patch adds a constructor to ThreadLocal to supply initial values using the new Supplier functional interface. Please review. This work was done by Jim Gish. http://cr.openjdk.java.net/~akhil/8003246.0/webrev/ http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8003246 Thanks

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-05 Thread Mike Duigou
Looks good to me. Mike On Dec 5 2012, at 11:39 , Akhil Arora wrote: This patch adds a constructor to ThreadLocal to supply initial values using the new Supplier functional interface. Please review. This work was done by Jim Gish. http://cr.openjdk.java.net/~akhil/8003246.0/webrev/

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-05 Thread Doug Lea
On 12/05/12 14:39, Akhil Arora wrote: This patch adds a constructor to ThreadLocal to supply initial values using the new Supplier functional interface. Please review. This work was done by Jim Gish. http://cr.openjdk.java.net/~akhil/8003246.0/webrev/

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-05 Thread Remi Forax
On 12/05/2012 08:51 PM, Doug Lea wrote: On 12/05/12 14:39, Akhil Arora wrote: This patch adds a constructor to ThreadLocal to supply initial values using the new Supplier functional interface. Please review. This work was done by Jim Gish. http://cr.openjdk.java.net/~akhil/8003246.0/webrev/

Re: RFR: 8003246: Add Supplier to ThreadLocal

2012-12-05 Thread David Holmes
On 6/12/2012 5:39 AM, Akhil Arora wrote: This patch adds a constructor to ThreadLocal to supply initial values using the new Supplier functional interface. Please review. This work was done by Jim Gish. http://cr.openjdk.java.net/~akhil/8003246.0/webrev/