TLAB Refills for x86

classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|

TLAB Refills for x86

JC Beyler
Hi all,

The TLAB and the inline contiguous allocations handling are different for
each architecture. On certain architectures, TLAB is never actually never
refilled (ref:  https://bugs.openjdk.java.net/browse/JDK-8190862).

The idea behind the implementation for x86 is to separate TLAB usage to
contiguous allocations in eden space.

Bug: https://bugs.openjdk.java.net/browse/JDK-8191027
WebRev: http://cr.openjdk.java.net/~rasbold/8191027/webrev.02/

Does anyone see any issues with this webrev?

Thanks for your help,
Jc
Reply | Threaded
Open this post in threaded view
|

Re: TLAB Refills for x86

Robbin Ehn
Hi JC,

Passes tier1-5 and performance testing ok.

Looks good, thanks for fixing!

/Robbin

On 2017-11-30 22:09, JC Beyler wrote:

> Hi all,
>
> The TLAB and the inline contiguous allocations handling are different for
> each architecture. On certain architectures, TLAB is never actually never
> refilled (ref:  https://bugs.openjdk.java.net/browse/JDK-8190862).
>
> The idea behind the implementation for x86 is to separate TLAB usage to
> contiguous allocations in eden space.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8191027
> WebRev: http://cr.openjdk.java.net/~rasbold/8191027/webrev.02/
>
> Does anyone see any issues with this webrev?
>
> Thanks for your help,
> Jc
>
Reply | Threaded
Open this post in threaded view
|

Re: TLAB Refills for x86

Thomas Schatzl
In reply to this post by JC Beyler
Hi,

On Thu, 2017-11-30 at 13:09 -0800, JC Beyler wrote:

> Hi all,
>
> The TLAB and the inline contiguous allocations handling are different
> for
> each architecture. On certain architectures, TLAB is never actually
> never
> refilled (ref:  https://bugs.openjdk.java.net/browse/JDK-8190862).
>
> The idea behind the implementation for x86 is to separate TLAB usage
> to
> contiguous allocations in eden space.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8191027
> WebRev: http://cr.openjdk.java.net/~rasbold/8191027/webrev.02/
>
> Does anyone see any issues with this webrev?

I only see a slight issue with the new comment on how it works: the
first time I read it, it seemed to me that step 1+2 are inclusive, not
exclusive as is implemented (there is step 4 which explains that it is
exclusive retroactively). But maybe it's just me.

Looks good otherwise.

Thomas

Reply | Threaded
Open this post in threaded view
|

Re: TLAB Refills for x86

Doerr, Martin
In reply to this post by JC Beyler
Hi,

the interpreter change looks good to me (except the comment change).
I've taken a look at C1 and C2 compilers.
C2 already does it as desired: It either uses TLAB or shared eden allocation (if supported).

Unfortunately, the C1 implementation in c1_Runtime1_x86/sparc still contains a path "try_eden" when TLAB allocation fails. (Also see MacroAssembler::tlab_refill). I think this should be adapted as well. Would you agree?

Best regards,
Martin


-----Original Message-----
From: hotspot-dev [mailto:[hidden email]] On Behalf Of Thomas Schatzl
Sent: Dienstag, 5. Dezember 2017 12:07
To: JC Beyler <[hidden email]>; [hidden email]
Subject: Re: TLAB Refills for x86

Hi,

On Thu, 2017-11-30 at 13:09 -0800, JC Beyler wrote:

> Hi all,
>
> The TLAB and the inline contiguous allocations handling are different
> for
> each architecture. On certain architectures, TLAB is never actually
> never
> refilled (ref:  https://bugs.openjdk.java.net/browse/JDK-8190862).
>
> The idea behind the implementation for x86 is to separate TLAB usage
> to
> contiguous allocations in eden space.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8191027
> WebRev: http://cr.openjdk.java.net/~rasbold/8191027/webrev.02/
>
> Does anyone see any issues with this webrev?

I only see a slight issue with the new comment on how it works: the
first time I read it, it seemed to me that step 1+2 are inclusive, not
exclusive as is implemented (there is step 4 which explains that it is
exclusive retroactively). But maybe it's just me.

Looks good otherwise.

Thomas

Reply | Threaded
Open this post in threaded view
|

Re: TLAB Refills for x86

Robbin Ehn
Hi Martin,

On 2017-12-05 13:12, Doerr, Martin wrote:
> Hi,
>
> the interpreter change looks good to me (except the comment change).
> I've taken a look at C1 and C2 compilers.
> C2 already does it as desired: It either uses TLAB or shared eden allocation (if supported).
>
> Unfortunately, the C1 implementation in c1_Runtime1_x86/sparc still contains a path "try_eden" when TLAB allocation fails. (Also see MacroAssembler::tlab_refill). I think this should be adapted as well. Would you agree?

The flag FastTLABRefill will be obsoleted in JDK 11 and we can then start removing (after Dec 14) the functionality.
In 10 we turned FastTLABRefill off by default and deprecated it. (FastTLABRefill was only actually on when using non-G1 and for C1 only)
(https://bugs.openjdk.java.net/browse/JDK-8190925)
This will be handled in a separate enhancement 'removing dead code'.

This (interpreter tlab refills) will not make 10, instead targeted for 11.
So in 11 we are aiming for a consistence behavior in all (not looked at gral) 'compilers'.

Thanks!

/Robbin

>
> Best regards,
> Martin
>
>
> -----Original Message-----
> From: hotspot-dev [mailto:[hidden email]] On Behalf Of Thomas Schatzl
> Sent: Dienstag, 5. Dezember 2017 12:07
> To: JC Beyler <[hidden email]>; [hidden email]
> Subject: Re: TLAB Refills for x86
>
> Hi,
>
> On Thu, 2017-11-30 at 13:09 -0800, JC Beyler wrote:
>> Hi all,
>>
>> The TLAB and the inline contiguous allocations handling are different
>> for
>> each architecture. On certain architectures, TLAB is never actually
>> never
>> refilled (ref:  https://bugs.openjdk.java.net/browse/JDK-8190862).
>>
>> The idea behind the implementation for x86 is to separate TLAB usage
>> to
>> contiguous allocations in eden space.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8191027
>> WebRev: http://cr.openjdk.java.net/~rasbold/8191027/webrev.02/
>>
>> Does anyone see any issues with this webrev?
>
> I only see a slight issue with the new comment on how it works: the
> first time I read it, it seemed to me that step 1+2 are inclusive, not
> exclusive as is implemented (there is step 4 which explains that it is
> exclusive retroactively). But maybe it's just me.
>
> Looks good otherwise.
>
> Thomas
>
Reply | Threaded
Open this post in threaded view
|

Re: TLAB Refills for x86

Doerr, Martin
In reply to this post by JC Beyler
Hi Robbin,

ok. Thanks for the explanation.

Best regards,
Martin


-----Original Message-----
From: Robbin Ehn [mailto:[hidden email]]
Sent: Dienstag, 5. Dezember 2017 13:52
To: Doerr, Martin <[hidden email]>; Thomas Schatzl <[hidden email]>; JC Beyler <[hidden email]>; [hidden email]
Subject: Re: TLAB Refills for x86

Hi Martin,

On 2017-12-05 13:12, Doerr, Martin wrote:
> Hi,
>
> the interpreter change looks good to me (except the comment change).
> I've taken a look at C1 and C2 compilers.
> C2 already does it as desired: It either uses TLAB or shared eden allocation (if supported).
>
> Unfortunately, the C1 implementation in c1_Runtime1_x86/sparc still contains a path "try_eden" when TLAB allocation fails. (Also see MacroAssembler::tlab_refill). I think this should be adapted as well. Would you agree?

The flag FastTLABRefill will be obsoleted in JDK 11 and we can then start removing (after Dec 14) the functionality.
In 10 we turned FastTLABRefill off by default and deprecated it. (FastTLABRefill was only actually on when using non-G1 and for C1 only)
(https://bugs.openjdk.java.net/browse/JDK-8190925)
This will be handled in a separate enhancement 'removing dead code'.

This (interpreter tlab refills) will not make 10, instead targeted for 11.
So in 11 we are aiming for a consistence behavior in all (not looked at gral) 'compilers'.

Thanks!

/Robbin

>
> Best regards,
> Martin
>
>
> -----Original Message-----
> From: hotspot-dev [mailto:[hidden email]] On Behalf Of Thomas Schatzl
> Sent: Dienstag, 5. Dezember 2017 12:07
> To: JC Beyler <[hidden email]>; [hidden email]
> Subject: Re: TLAB Refills for x86
>
> Hi,
>
> On Thu, 2017-11-30 at 13:09 -0800, JC Beyler wrote:
>> Hi all,
>>
>> The TLAB and the inline contiguous allocations handling are different
>> for
>> each architecture. On certain architectures, TLAB is never actually
>> never
>> refilled (ref:  https://bugs.openjdk.java.net/browse/JDK-8190862).
>>
>> The idea behind the implementation for x86 is to separate TLAB usage
>> to
>> contiguous allocations in eden space.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8191027
>> WebRev: http://cr.openjdk.java.net/~rasbold/8191027/webrev.02/
>>
>> Does anyone see any issues with this webrev?
>
> I only see a slight issue with the new comment on how it works: the
> first time I read it, it seemed to me that step 1+2 are inclusive, not
> exclusive as is implemented (there is step 4 which explains that it is
> exclusive retroactively). But maybe it's just me.
>
> Looks good otherwise.
>
> Thomas
>
Reply | Threaded
Open this post in threaded view
|

Re: TLAB Refills for x86

JC Beyler
Hi all,

I added a reword of the comment (I could also remove it so let me know):
http://cr.openjdk.java.net/~rasbold/8191027/webrev.03/

Let me know if it is better!

Thanks,
Jc

On Tue, Dec 5, 2017 at 4:59 AM, Doerr, Martin <[hidden email]> wrote:

> Hi Robbin,
>
> ok. Thanks for the explanation.
>
> Best regards,
> Martin
>
>
> -----Original Message-----
> From: Robbin Ehn [mailto:[hidden email]]
> Sent: Dienstag, 5. Dezember 2017 13:52
> To: Doerr, Martin <[hidden email]>; Thomas Schatzl <
> [hidden email]>; JC Beyler <[hidden email]>;
> [hidden email]
> Subject: Re: TLAB Refills for x86
>
> Hi Martin,
>
> On 2017-12-05 13:12, Doerr, Martin wrote:
> > Hi,
> >
> > the interpreter change looks good to me (except the comment change).
> > I've taken a look at C1 and C2 compilers.
> > C2 already does it as desired: It either uses TLAB or shared eden
> allocation (if supported).
> >
> > Unfortunately, the C1 implementation in c1_Runtime1_x86/sparc still
> contains a path "try_eden" when TLAB allocation fails. (Also see
> MacroAssembler::tlab_refill). I think this should be adapted as well. Would
> you agree?
>
> The flag FastTLABRefill will be obsoleted in JDK 11 and we can then start
> removing (after Dec 14) the functionality.
> In 10 we turned FastTLABRefill off by default and deprecated it.
> (FastTLABRefill was only actually on when using non-G1 and for C1 only)
> (https://bugs.openjdk.java.net/browse/JDK-8190925)
> This will be handled in a separate enhancement 'removing dead code'.
>
> This (interpreter tlab refills) will not make 10, instead targeted for 11.
> So in 11 we are aiming for a consistence behavior in all (not looked at
> gral) 'compilers'.
>
> Thanks!
>
> /Robbin
>
> >
> > Best regards,
> > Martin
> >
> >
> > -----Original Message-----
> > From: hotspot-dev [mailto:[hidden email]] On
> Behalf Of Thomas Schatzl
> > Sent: Dienstag, 5. Dezember 2017 12:07
> > To: JC Beyler <[hidden email]>; [hidden email]
> > Subject: Re: TLAB Refills for x86
> >
> > Hi,
> >
> > On Thu, 2017-11-30 at 13:09 -0800, JC Beyler wrote:
> >> Hi all,
> >>
> >> The TLAB and the inline contiguous allocations handling are different
> >> for
> >> each architecture. On certain architectures, TLAB is never actually
> >> never
> >> refilled (ref:  https://bugs.openjdk.java.net/browse/JDK-8190862).
> >>
> >> The idea behind the implementation for x86 is to separate TLAB usage
> >> to
> >> contiguous allocations in eden space.
> >>
> >> Bug: https://bugs.openjdk.java.net/browse/JDK-8191027
> >> WebRev: http://cr.openjdk.java.net/~rasbold/8191027/webrev.02/
> >>
> >> Does anyone see any issues with this webrev?
> >
> > I only see a slight issue with the new comment on how it works: the
> > first time I read it, it seemed to me that step 1+2 are inclusive, not
> > exclusive as is implemented (there is step 4 which explains that it is
> > exclusive retroactively). But maybe it's just me.
> >
> > Looks good otherwise.
> >
> > Thomas
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: TLAB Refills for x86

Thomas Schatzl
Hi,

On Thu, 2017-12-07 at 21:49 -0800, JC Beyler wrote:
> Hi all,
>
> I added a reword of the comment (I could also remove it so let me
> know):
> http://cr.openjdk.java.net/~rasbold/8191027/webrev.03/
>
> Let me know if it is better!

  my suggestion:

3872   // Allocate the instance:
3873   // 1) If TLAB is enabled:
3874   //  a) Try to allocate in the TLAB
3875   //  b) If fails, go to the slow path.
3876   //    Else If inline contiguous allocations are enabled:
3878   //  i) Try to allocate in eden
3879   //  ii) If fails due to heap end, go to slow path.
3880   // 2) If TLAB is enabled OR inline contiguous is enabled:
3881   //  a) Initialize the allocation
3882   //  b) Exit.
3883   // 3) Go to slow path.
3884

Same idea for the SPARC change.

Thanks,
  Thomas

Reply | Threaded
Open this post in threaded view
|

Re: TLAB Refills for x86

JC Beyler
Hi Thomas,

Done here:
http://cr.openjdk.java.net/~rasbold/8191027/webrev.04/

Let me know what you think,
Jc

On Mon, Dec 11, 2017 at 3:06 AM, Thomas Schatzl
<[hidden email]> wrote:

> Hi,
>
> On Thu, 2017-12-07 at 21:49 -0800, JC Beyler wrote:
>> Hi all,
>>
>> I added a reword of the comment (I could also remove it so let me
>> know):
>> http://cr.openjdk.java.net/~rasbold/8191027/webrev.03/
>>
>> Let me know if it is better!
>
>   my suggestion:
>
> 3872   // Allocate the instance:
> 3873   // 1) If TLAB is enabled:
> 3874   //  a) Try to allocate in the TLAB
> 3875   //  b) If fails, go to the slow path.
> 3876   //    Else If inline contiguous allocations are enabled:
> 3878   //  i) Try to allocate in eden
> 3879   //  ii) If fails due to heap end, go to slow path.
> 3880   // 2) If TLAB is enabled OR inline contiguous is enabled:
> 3881   //  a) Initialize the allocation
> 3882   //  b) Exit.
> 3883   // 3) Go to slow path.
> 3884
>
> Same idea for the SPARC change.
>
> Thanks,
>   Thomas
>
Reply | Threaded
Open this post in threaded view
|

Re: TLAB Refills for x86

Thomas Schatzl
Hi,

On Thu, 2017-12-14 at 08:04 -0800, JC Beyler wrote:
> Hi Thomas,
>
> Done here:
> http://cr.openjdk.java.net/~rasbold/8191027/webrev.04/
>
> Let me know what you think,
> Jc


I only looked at the comment, which looks good now :)

Thanks,
  Thomas
Reply | Threaded
Open this post in threaded view
|

Re: TLAB Refills for x86

JC Beyler
Thanks :)

Forgot to say that I removed the numbering, the indentation seemed
enough and the numbering did make it look confusing... even to me ;-)
Jc

On Thu, Dec 14, 2017 at 8:08 AM, Thomas Schatzl
<[hidden email]> wrote:

> Hi,
>
> On Thu, 2017-12-14 at 08:04 -0800, JC Beyler wrote:
>> Hi Thomas,
>>
>> Done here:
>> http://cr.openjdk.java.net/~rasbold/8191027/webrev.04/
>>
>> Let me know what you think,
>> Jc
>
>
> I only looked at the comment, which looks good now :)
>
> Thanks,
>   Thomas
Reply | Threaded
Open this post in threaded view
|

Re: TLAB Refills for x86

Thomas Schatzl
Hi,

On Thu, 2017-12-14 at 08:10 -0800, JC Beyler wrote:
> Thanks :)
>
> Forgot to say that I removed the numbering, the indentation seemed
> enough and the numbering did make it look confusing... even to me ;-)

 I did notice that :) I agree.

Thomas