Re: RFR: 8003246: Add Supplier to ThreadLocal
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 methods in java.lang
-Chris.
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 '\.access\$00'
Technically, javac also generate constructor accessors that doesn't
start by access00 but this should catch most of the generated accessors.
once changeset for 8003246 will be pushed, I will send two patches one
for java.lang and one for java.util.
-Chris
Rémi
On 07/12/2012 14:27, Doug Lea wrote:
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 annoying ones that kick in on any
compile of anything involving ThreadLocals (try running with
-XX:+PrintCompilation). To remove, replace private with
final for methods, and just kill private for fields.
-Doug
Re: RFR: 8003246: Add Supplier to ThreadLocal
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 filed 8004797: Remove superfluous access$000 methods in java.lang -Chris. thanks. Rémi
Re: RFR: 8003246: Add Supplier to ThreadLocal
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 annoying ones that kick in on any compile of anything involving ThreadLocals (try running with -XX:+PrintCompilation). To remove, replace private with final for methods, and just kill private for fields. -Doug
Re: RFR: 8003246: Add Supplier to ThreadLocal
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 '\.access\$00'
-Chris
On 07/12/2012 14:27, Doug Lea wrote:
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 annoying ones that kick in on any
compile of anything involving ThreadLocals (try running with
-XX:+PrintCompilation). To remove, replace private with
final for methods, and just kill private for fields.
-Doug
Re: RFR: 8003246: Add Supplier to ThreadLocal
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
build/solaris-i586/classes/java/util
: find . -name *.class -exec javap -v {} \; | grep '\.access\$00'
Technically, javac also generate constructor accessors that doesn't
start by access00 but this should catch most of the generated accessors.
once changeset for 8003246 will be pushed, I will send two patches one
for java.lang and one for java.util.
-Chris
Rémi
On 07/12/2012 14:27, Doug Lea wrote:
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 annoying ones that kick in on any
compile of anything involving ThreadLocals (try running with
-XX:+PrintCompilation). To remove, replace private with
final for methods, and just kill private for fields.
-Doug
Re: RFR: 8003246: Add Supplier to ThreadLocal
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. http://cr.openjdk.java.net/~akhil/8003246.0/webrev/ http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8003246 Has anyone actually established that this is a useful addition to make? What is the acceptance criteria for adding these new mechanisms to existing classes? Was there a CCC request for this? We need to be very wary of unnecessary bloat. It's a necessary bloat :) There are two good reasons to provide a new ThreadLocal(() - initialValue), if all goes as planned, every Supplier will share the same class so instead of having one class by thread local that want to specify an initialValue, we will have only 2 classes (or 3 if the ThreadLocal that takes a Supplier is a subclass) in memory. Also, letting people to subclass ThreadLocal is not a good idea because if in one location someone decide to override get() to by example add a logging code (I've seen a similar problem) then suddenly all the codes that use ThreadLocal.get() will not be able to inline it. Given that too many things are done using ThreadLocal in JEE container, having a way to provide an initial value without subclassing ThreadLocal is a good idea. David Rémi
Re: RFR: 8003246: Add Supplier to ThreadLocal
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. http://cr.openjdk.java.net/~akhil/8003246.0/webrev/ http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8003246 ... There are two good reasons to provide a new ThreadLocal(() - initialValue), if all goes as planned, every Supplier will share the same class so instead of having one class by thread local that want to specify an initialValue, we will have only 2 classes (or 3 if the ThreadLocal that takes a Supplier is a subclass) in memory. 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 benefit of, by not touching ThreadLocal, guaranteeing that it is time/space-neutral for other existing uses. Which also means that any future time/space improvements in ThreadLocal will not need to be constrained by this change. Jim/Akhil, could you please redo it this way? Also, letting people to subclass ThreadLocal is not a good idea because if in one location someone decide to override get() to by example add a logging code (I've seen a similar problem) then suddenly all the codes that use ThreadLocal.get() will not be able to inline it. (Yes. We see this when our (very heavy) uses of ThreadLocal inside j.u.c. go from fast to slow mode due to overrides in other unrelated ThreadLocal classes in application code. I've many times considered introducing a JDK-internal variant of ThreadLocal to protect against such things. Possibly even one with only a finite fixed capacity, that would allow further speed ups. Or maybe just introducing say a dozen extra dedicated fields in class Thread, making it optimally fast at the expense of non-extensibility.) -Doug
Re: RFR: 8003246: Add Supplier to ThreadLocal
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 understand what Rémi and you are saying. Thanks Sent from my phone On Dec 6, 2012 6:33 AM, Doug Lea d...@cs.oswego.edu wrote: 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. http://cr.openjdk.java.net/~**akhil/8003246.0/webrev/http://cr.openjdk.java.net/~akhil/8003246.0/webrev/ http://bugs.sun.com/**bugdatabase/view_bug.do?bug_**id=8003246http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8003246 ... There are two good reasons to provide a new ThreadLocal(() - initialValue), if all goes as planned, every Supplier will share the same class so instead of having one class by thread local that want to specify an initialValue, we will have only 2 classes (or 3 if the ThreadLocal that takes a Supplier is a subclass) in memory. 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 benefit of, by not touching ThreadLocal, guaranteeing that it is time/space-neutral for other existing uses. Which also means that any future time/space improvements in ThreadLocal will not need to be constrained by this change. Jim/Akhil, could you please redo it this way? Also, letting people to subclass ThreadLocal is not a good idea because if in one location someone decide to override get() to by example add a logging code (I've seen a similar problem) then suddenly all the codes that use ThreadLocal.get() will not be able to inline it. (Yes. We see this when our (very heavy) uses of ThreadLocal inside j.u.c. go from fast to slow mode due to overrides in other unrelated ThreadLocal classes in application code. I've many times considered introducing a JDK-internal variant of ThreadLocal to protect against such things. Possibly even one with only a finite fixed capacity, that would allow further speed ups. Or maybe just introducing say a dozen extra dedicated fields in class Thread, making it optimally fast at the expense of non-extensibility.) -Doug
Re: RFR: 8003246: Add Supplier to ThreadLocal
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 care about) doesn't change after other subclasses are loaded. Would be good if someone can correct that if it's inaccurate. Apologies if this is slightly hijacking the thread, but it's Remi's fault :). Sent from my phone On Dec 6, 2012 7:03 AM, Doug Lea d...@cs.oswego.edu wrote: On 12/06/12 06:56, Vitaly Davidovich wrote: 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 understand what Rémi and you are saying. The possible outcomes are fairly non-deterministic, depending on hotspot's mood about recompiles, tiered-compile interactions, method size, Amddahl's law interactions, phase of moon, etc. (In j.u.c, we have learned that our users appreciate things being predictably fast enough rather than being unpredictably sometimes even faster but often slower. So when we see such cases, as with ThreadLocal, they get added to todo list.) -Doug
Re: RFR: 8003246: Add Supplier to ThreadLocal
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 benefit of, by not touching ThreadLocal, guaranteeing that it is time/space-neutral for other existing uses. Which also means that any future time/space improvements in ThreadLocal will not need to be constrained by this change. Jim/Akhil, could you please redo it this way? redone - http://cr.openjdk.java.net/~akhil/8003246.1/webrev/ Thanks
Re: RFR: 8003246: Add Supplier to ThreadLocal
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.
Should these statements defer to the text in initialValue? Or maybe just
leave out the additional text in the no-args constructor, and have
withInitial() just describe its implementation? My concern is with the
omission of the behavior of set(), and the term 'initial value' could be
misinterpreted.
Anyway, I'm relatively happy with this, let's see what others think.
-Chris.
On 12/06/2012 05:01 PM, Akhil Arora wrote:
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 benefit of, by not touching ThreadLocal,
guaranteeing that it is time/space-neutral for other existing
uses. Which also means that any future time/space improvements
in ThreadLocal will not need to be constrained by this change.
Jim/Akhil, could you please redo it this way?
redone - http://cr.openjdk.java.net/~akhil/8003246.1/webrev/
Thanks
Re: RFR: 8003246: Add Supplier to ThreadLocal
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, stacktraces, etc,
and the default $ name will be confusing. As in:
static final class SuppliedThreadLocalT extends ThreadLocalT ..
public static T ThreadLocalT withInitial(Supplier? extends T supplier) {
return new SuppliedThreadLocalT(supplier);
}
Re: RFR: 8003246: Add Supplier to ThreadLocal
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:
static final class SuppliedThreadLocalT extends ThreadLocalT ..
public static T ThreadLocalT withInitial(Supplier? extends T
supplier) {
return new SuppliedThreadLocalT(supplier);
}
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
Thanks
Re: RFR: 8003246: Add Supplier to ThreadLocal
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
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
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); }
@Override
public final void remove() { super.remove(); }
}
just use static type FastThreadLocal everywhere in code.
I tried it and it works.
Regards, Peter
On 12/06/2012 01:03 PM, Doug Lea wrote:
On 12/06/12 06:56, Vitaly Davidovich wrote:
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
understand what
Rémi and you are saying.
The possible outcomes are fairly non-deterministic, depending
on hotspot's mood about recompiles, tiered-compile interactions,
method size, Amddahl's law interactions, phase of moon, etc.
(In j.u.c, we have learned that our users appreciate things
being predictably fast enough rather than being
unpredictably sometimes even faster but often slower.
So when we see such cases, as with ThreadLocal, they get added
to todo list.)
-Doug
Re: RFR: 8003246: Add Supplier to ThreadLocal
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. -Doug Yes, great ! 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). cheers, Rémi
Re: RFR: 8003246: Add Supplier to ThreadLocal
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 set(T value) { super.set(value); }
@Override
public final void remove() { super.remove(); }
}
just use static type FastThreadLocal everywhere in code.
I tried it and it works.
No, there is no way to have such guarantee, here, it works either
because the only class ThreadLocal you load is FastThreadLocal or
because the VM has profiled the callsite see that you only use
FastThreadLocal for a specific instruction.
Regards, Peter
cheers,
Rémi
On 12/06/2012 01:03 PM, Doug Lea wrote:
On 12/06/12 06:56, Vitaly Davidovich wrote:
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
understand what
Rémi and you are saying.
The possible outcomes are fairly non-deterministic, depending
on hotspot's mood about recompiles, tiered-compile interactions,
method size, Amddahl's law interactions, phase of moon, etc.
(In j.u.c, we have learned that our users appreciate things
being predictably fast enough rather than being
unpredictably sometimes even faster but often slower.
So when we see such cases, as with ThreadLocal, they get added
to todo list.)
-Doug
Re: RFR: 8003246: Add Supplier to ThreadLocal
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 restored assuming call site type profile (of the ones we care about) doesn't change after other subclasses are loaded. Would be good if someone can correct that if it's inaccurate. Apologies if this is slightly hijacking the thread, but it's Remi's fault :). :) Usually, it's inline just fine because the code just stores the ThreadLocal in a static field, the VM profiles the code and you get the fast path. But some codes use a ThreadLocal in a non static field or use a map of ThreadLocal so the profile becomes bloated, and there is no inlining anymore. If all loaded codes always use the same ThreadLocal (using ThreadLocal.withSupplier()) then, the call to the supplier is not inlined but you don't care because it's not part of the fast path of get. cheers, Rémi Sent from my phone On Dec 6, 2012 7:03 AM, Doug Lea d...@cs.oswego.edu wrote: On 12/06/12 06:56, Vitaly Davidovich wrote: 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 understand what Rémi and you are saying. The possible outcomes are fairly non-deterministic, depending on hotspot's mood about recompiles, tiered-compile interactions, method size, Amddahl's law interactions, phase of moon, etc. (In j.u.c, we have learned that our users appreciate things being predictably fast enough rather than being unpredictably sometimes even faster but often slower. So when we see such cases, as with ThreadLocal, they get added to todo list.) -Doug
Re: RFR: 8003246: Add Supplier to ThreadLocal
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). http://cr.openjdk.java.net/~akhil/8003246.3/webrev/ also incorporates a nitpick from Mike - Objects.requireNonNull returns its arg Thanks - nitpicking welcome :)
Re: RFR: 8003246: Add Supplier to ThreadLocal
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 the given Supplier needs to be thread-safe? -Alan.
Re: RFR: 8003246: Add Supplier to ThreadLocal
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(); }
@Override
public final void set(T value) { super.set(value); }
@Override
public final void remove() { super.remove(); }
}
just use static type FastThreadLocal everywhere in code.
I tried it and it works.
No, there is no way to have such guarantee, here, it works either
because the only class ThreadLocal you load is FastThreadLocal or
because the VM has profiled the callsite see that you only use
FastThreadLocal for a specific instruction.
Nothing is certain but death and taxes, I agree.
But think deeper, Remi!
How do you explain the following test:
public class ThreadLocalTest {
static class Int { int value; }
static class TL0 extends ThreadLocalInt {}
static class TL1 extends ThreadLocalInt { public Int get() {
return super.get(); } }
static class TL2 extends ThreadLocalInt { public Int get() {
return super.get(); } }
static class TL3 extends ThreadLocalInt { public Int get() {
return super.get(); } }
static class TL4 extends ThreadLocalInt { public Int get() {
return super.get(); } }
static long doTest(ThreadLocalInt tl) {
long t0 = System.nanoTime();
for (int i = 0; i 1; i++)
tl.get().value++;
return System.nanoTime() - t0;
}
static long doTest(FastThreadLocalInt tl) {
long t0 = System.nanoTime();
for (int i = 0; i 1; i++)
tl.get().value++;
return System.nanoTime() - t0;
}
static long test0(ThreadLocalInt tl) {
if (tl instanceof FastThreadLocal)
return doTest((FastThreadLocalInt)tl);
else
return doTest(tl);
}
static void test(ThreadLocalInt tl) {
tl.set(new Int());
System.out.print(tl.getClass().getName() + :);
for (int i = 0; i 8; i++)
System.out.print( + test0(tl));
System.out.println();
}
public static void main(String[] args) {
TL0 tl0 = new TL0();
test(tl0);
test(new TL1());
test(new TL2());
test(new TL3());
test(new TL4());
test(tl0);
}
}
Which prints the following (demonstrating almost 2x slowdown of TL0 -
last line compared to first):
test.ThreadLocalTest$TL0: 342716421 326105315 300744544 300654890
300726346 300752009 300700781 300735651
test.ThreadLocalTest$TL1: 321424139 312128166 312173383 312125203
312142144 312150949 316760957 313393554
test.ThreadLocalTest$TL2: 525661886 524169413 524184405 524215685
524162050 524400364 524174966 454370228
test.ThreadLocalTest$TL3: 472042229 471071328 464387909 468047355
464795171 464466481 464449567 464365974
test.ThreadLocalTest$TL4: 459651686 454142365 454129481 454180718
454217277 454109611 454119988 456978405
test.ThreadLocalTest$TL0: 582252322 582773455 582612509 582753610
582626360 582852195 582805654 582598285
Now with a simple change of:
static class TL0 extends FastThreadLocalInt {}
...the same test prints:
test.ThreadLocalTest$TL0: 330722181 325823711 301171182 309992192
321868979 308111417 303806979 300612033
test.ThreadLocalTest$TL1: 330263857 326448062 300607081 300575641
307442821 300616794 300548457 303462898
test.ThreadLocalTest$TL2: 319627165 311309477 311465815 311279612
311294427 311315803 311470291 311293823
test.ThreadLocalTest$TL3: 526849874 524209792 524421574 524166747
524396011 524163313 524395641 524165429
test.ThreadLocalTest$TL4: 464963126 455172216 455466304 455245487
455368318 455093735 455125038 455317375
test.ThreadLocalTest$TL0: 300472239 300695398 300480230 303459397
300451419 300679904 300445717 300451166
And that's very repeatable! Try it for yourself (on JDK8 of course).
Regards, Peter
Regards, Peter
cheers,
Rémi
On 12/06/2012 01:03 PM, Doug Lea wrote:
On 12/06/12 06:56, Vitaly Davidovich wrote:
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
understand what
Rémi and you are saying.
The possible outcomes are fairly non-deterministic, depending
on hotspot's mood about recompiles, tiered-compile interactions,
method size, Amddahl's law interactions, phase of moon, etc.
(In j.u.c, we have learned that our users appreciate things
being predictably fast enough rather than being
unpredictably sometimes even faster but often slower.
So when we see such cases, as with ThreadLocal, they get added
to todo list.)
-Doug
Re: RFR: 8003246: Add Supplier to ThreadLocal
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 class TL1 extends ThreadLocalInt { public Int get() {
return super.get(); } }
static class TL2 extends ThreadLocalInt { public Int get() {
return super.get(); } }
static class TL3 extends ThreadLocalInt { public Int get() {
return super.get(); } }
static class TL4 extends ThreadLocalInt { public Int get() {
return super.get(); } }
static long doTest(ThreadLocalInt tl) {
long t0 = System.nanoTime();
for (int i = 0; i 1; i++)
tl.get().value++;
return System.nanoTime() - t0;
}
static long doTest0(ThreadLocalInt tl) {
long t0 = System.nanoTime();
for (int i = 0; i 1; i++)
tl.get().value++;
return System.nanoTime() - t0;
}
static long test0(ThreadLocalInt tl) {
if (tl instanceof TL0)
return doTest0(tl);
else
return doTest(tl);
}
But I think that deoptimizations that Dough is talking about might be
prevented by using the following variant of TL:
public class FastThreadLocalT extends ThreadLocalT {
public final T getFast() { return super.get(); }
public final void setFast(T value) { super.set(value); }
public final void removeFast() { super.remove(); }
}
and invoking the fast methods in code.
Right?
Regards, Peter
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 final T get() { return super.get(); }
@Override
public final void set(T value) { super.set(value); }
@Override
public final void remove() { super.remove(); }
}
just use static type FastThreadLocal everywhere in code.
I tried it and it works.
No, there is no way to have such guarantee, here, it works either
because the only class ThreadLocal you load is FastThreadLocal or
because the VM has profiled the callsite see that you only use
FastThreadLocal for a specific instruction.
Nothing is certain but death and taxes, I agree.
But think deeper, Remi!
How do you explain the following test:
public class ThreadLocalTest {
static class Int { int value; }
static class TL0 extends ThreadLocalInt {}
static class TL1 extends ThreadLocalInt { public Int get() {
return super.get(); } }
static class TL2 extends ThreadLocalInt { public Int get() {
return super.get(); } }
static class TL3 extends ThreadLocalInt { public Int get() {
return super.get(); } }
static class TL4 extends ThreadLocalInt { public Int get() {
return super.get(); } }
static long doTest(ThreadLocalInt tl) {
long t0 = System.nanoTime();
for (int i = 0; i 1; i++)
tl.get().value++;
return System.nanoTime() - t0;
}
static long doTest(FastThreadLocalInt tl) {
long t0 = System.nanoTime();
for (int i = 0; i 1; i++)
tl.get().value++;
return System.nanoTime() - t0;
}
static long test0(ThreadLocalInt tl) {
if (tl instanceof FastThreadLocal)
return doTest((FastThreadLocalInt)tl);
else
return doTest(tl);
}
static void test(ThreadLocalInt tl) {
tl.set(new Int());
System.out.print(tl.getClass().getName() + :);
for (int i = 0; i 8; i++)
System.out.print( + test0(tl));
System.out.println();
}
public static void main(String[] args) {
TL0 tl0 = new TL0();
test(tl0);
test(new TL1());
test(new TL2());
test(new TL3());
test(new TL4());
test(tl0);
}
}
Which prints the following (demonstrating almost 2x slowdown of TL0 -
last line compared to first):
test.ThreadLocalTest$TL0: 342716421 326105315 300744544 300654890
300726346 300752009 300700781 300735651
test.ThreadLocalTest$TL1: 321424139 312128166 312173383 312125203
312142144 312150949 316760957 313393554
test.ThreadLocalTest$TL2: 525661886 524169413 524184405 524215685
524162050 524400364 524174966 454370228
test.ThreadLocalTest$TL3: 472042229 471071328 464387909 468047355
464795171 464466481 464449567 464365974
test.ThreadLocalTest$TL4: 459651686 454142365 454129481 454180718
454217277 454109611 454119988 456978405
test.ThreadLocalTest$TL0: 582252322 582773455 582612509 582753610
582626360 582852195 582805654 582598285
Now with a simple change of:
static class TL0 extends FastThreadLocalInt {}
...the same test prints:
test.ThreadLocalTest$TL0: 330722181 325823711
Re: RFR: 8003246: Add Supplier to ThreadLocal
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 final T get() { return super.get(); }
@Override
public final void set(T value) { super.set(value); }
@Override
public final void remove() { super.remove(); }
}
just use static type FastThreadLocal everywhere in code.
I tried it and it works.
No, there is no way to have such guarantee, here, it works either
because the only class ThreadLocal you load is FastThreadLocal or
because the VM has profiled the callsite see that you only use
FastThreadLocal for a specific instruction.
Nothing is certain but death and taxes, I agree.
But think deeper, Remi!
I try, i try, i've trouble to understand what you mean
How do you explain the following test:
public class ThreadLocalTest {
static class Int { int value; }
static class TL0 extends ThreadLocalInt {}
static class TL1 extends ThreadLocalInt { public Int get() {
return super.get(); } }
static class TL2 extends ThreadLocalInt { public Int get() {
return super.get(); } }
static class TL3 extends ThreadLocalInt { public Int get() {
return super.get(); } }
static class TL4 extends ThreadLocalInt { public Int get() {
return super.get(); } }
static long doTest(ThreadLocalInt tl) {
long t0 = System.nanoTime();
for (int i = 0; i 1; i++)
tl.get().value++; //
callsite 1
return System.nanoTime() - t0;
}
here, tl.get() (callsite 1) is called with TL0, TL1, TL2, TL3 and TL4,
do tl.get() is a polymorphic call
static long doTest(FastThreadLocalInt tl) {
long t0 = System.nanoTime();
for (int i = 0; i 1; i++)
tl.get().value++; // callsite 2
return System.nanoTime() - t0;
}
if TL0 is defined like as a FastThreadLocal. doTest is called and here
tl.get() (callsite 2) is only called with a FastThreadLocal, so the call
is inlined.
static long test0(ThreadLocalInt tl) {
if (tl instanceof FastThreadLocal)
return doTest((FastThreadLocalInt)tl);
else
return doTest(tl);
}
static void test(ThreadLocalInt tl) {
tl.set(new Int());
System.out.print(tl.getClass().getName() + :);
for (int i = 0; i 8; i++)
System.out.print( + test0(tl));
System.out.println();
}
public static void main(String[] args) {
TL0 tl0 = new TL0();
test(tl0);
test(new TL1());
test(new TL2());
test(new TL3());
test(new TL4());
test(tl0);
}
}
Which prints the following (demonstrating almost 2x slowdown of TL0 -
last line compared to first):
test.ThreadLocalTest$TL0: 342716421 326105315 300744544 300654890
300726346 300752009 300700781 300735651
test.ThreadLocalTest$TL1: 321424139 312128166 312173383 312125203
312142144 312150949 316760957 313393554
test.ThreadLocalTest$TL2: 525661886 524169413 524184405 524215685
524162050 524400364 524174966 454370228
test.ThreadLocalTest$TL3: 472042229 471071328 464387909 468047355
464795171 464466481 464449567 464365974
test.ThreadLocalTest$TL4: 459651686 454142365 454129481 454180718
454217277 454109611 454119988 456978405
test.ThreadLocalTest$TL0: 582252322 582773455 582612509 582753610
582626360 582852195 582805654 582598285
Now with a simple change of:
static class TL0 extends FastThreadLocalInt {}
...the same test prints:
test.ThreadLocalTest$TL0: 330722181 325823711 301171182 309992192
321868979 308111417 303806979 300612033
test.ThreadLocalTest$TL1: 330263857 326448062 300607081 300575641
307442821 300616794 300548457 303462898
test.ThreadLocalTest$TL2: 319627165 311309477 311465815 311279612
311294427 311315803 311470291 311293823
test.ThreadLocalTest$TL3: 526849874 524209792 524421574 524166747
524396011 524163313 524395641 524165429
test.ThreadLocalTest$TL4: 464963126 455172216 455466304 455245487
455368318 455093735 455125038 455317375
test.ThreadLocalTest$TL0: 300472239 300695398 300480230 303459397
300451419 300679904 300445717 300451166
And that's very repeatable! Try it for yourself (on JDK8 of course).
Regards, Peter
so to summarize, the tl.get() in doTest(ThreadLocal) is polymorphic and
the one in doTest(FastThreadLocal) is monomorphic thus inlinable.
It has nothing to do with the fact that FastThreadLocal override get,
set and remove.
You can comment the methods inside FastThreadLocal to see that it change
nothing.
regards,
Rémi
Re: RFR: 8003246: Add Supplier to ThreadLocal
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 Rémi once said :) -Jeff
Re: RFR: 8003246: Add Supplier to ThreadLocal
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 bytecode). (*) If I correctly undertood what Rémi once said :) It's true only if the field is accessed ouside the inner class but inside the enclosing class (or the enclosing class of the enclosing class). 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. -Jeff Rémi
Re: RFR: 8003246: Add Supplier to ThreadLocal
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/ http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8003246 Thanks
Re: RFR: 8003246: Add Supplier to ThreadLocal
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/ http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8003246 Thanks I see that EVERY ThreadLocal now has an extra field in case it is used with a Supplier. I expect that those web frameworks that create thousands of ThreadLocals (not just instances of Threadlocals) will see a measurable space increase. has anyone measure the impact? Did anyone consider instead defining a SuppliedThreadLocal subclass (or some better name) to isolate the impact? -Doug
Re: RFR: 8003246: Add Supplier to ThreadLocal
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/ http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8003246 Thanks I see that EVERY ThreadLocal now has an extra field in case it is used with a Supplier. I expect that those web frameworks that create thousands of ThreadLocals (not just instances of Threadlocals) will see a measurable space increase. has anyone measure the impact? Did anyone consider instead defining a SuppliedThreadLocal subclass (or some better name) to isolate the impact? The class doesn't even to have a name, one can add a static factory method in ThreadLocal and use an anonymous class to implement it. The other problem is that SupplierT should be replaced by Supplier? extends T everywhere in the code. -Doug Rémi
Re: RFR: 8003246: Add Supplier to ThreadLocal
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/ http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8003246 Has anyone actually established that this is a useful addition to make? What is the acceptance criteria for adding these new mechanisms to existing classes? Was there a CCC request for this? We need to be very wary of unnecessary bloat. David Thanks
