RFR 8191985: Tlab refills for ARM

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

RFR 8191985: Tlab refills for ARM

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 Arm is the same as for
Sparc/X86/Aarch64 and is to do one thing:
  - separate TLAB usage to contiguous allocations in eden space.

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

Does anyone see any issues with this webrev? Could someone test/verify
it please?

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

Re: RFR 8191985: Tlab refills for ARM

Robbin Ehn
Hi JC,

We do not longer have the ability to build and test arm32. And none seem to be picking this port up.
Only eyeballing this to looks good!

/Robbin

On 2017-12-14 17:08, 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 Arm is the same as for
> Sparc/X86/Aarch64 and is to do one thing:
>    - separate TLAB usage to contiguous allocations in eden space.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8191985
> WebRev: http://cr.openjdk.java.net/~rasbold/8191985/webrev.03/
>
> Does anyone see any issues with this webrev? Could someone test/verify
> it please?
>
> Thanks for your help,
> Jc
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8191985: Tlab refills for ARM

JC Beyler
Hi all,

Thanks Robbin for the eye-balling.

I did update the webrev to here:
http://cr.openjdk.java.net/~jcbeyler/8191985/webrev.01/

Let me know what you think,
Jc

On Tue, Dec 19, 2017 at 2:24 AM, Robbin Ehn <[hidden email]> wrote:

> Hi JC,
>
> We do not longer have the ability to build and test arm32. And none seem to
> be picking this port up.
> Only eyeballing this to looks good!
>
> /Robbin
>
>
> On 2017-12-14 17:08, 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 Arm is the same as for
>> Sparc/X86/Aarch64 and is to do one thing:
>>    - separate TLAB usage to contiguous allocations in eden space.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8191985
>> WebRev: http://cr.openjdk.java.net/~rasbold/8191985/webrev.03/
>>
>> Does anyone see any issues with this webrev? Could someone test/verify
>> it please?
>>
>> Thanks for your help,
>> Jc
>>
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8191985: Tlab refills for ARM

Robbin Ehn
Hi JC,

Looks good to me.

Can someone else please eyeball this?

/Robbin

On 2017-12-20 05:39, JC Beyler wrote:

> Hi all,
>
> Thanks Robbin for the eye-balling.
>
> I did update the webrev to here:
> http://cr.openjdk.java.net/~jcbeyler/8191985/webrev.01/
>
> Let me know what you think,
> Jc
>
> On Tue, Dec 19, 2017 at 2:24 AM, Robbin Ehn <[hidden email]> wrote:
>> Hi JC,
>>
>> We do not longer have the ability to build and test arm32. And none seem to
>> be picking this port up.
>> Only eyeballing this to looks good!
>>
>> /Robbin
>>
>>
>> On 2017-12-14 17:08, 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 Arm is the same as for
>>> Sparc/X86/Aarch64 and is to do one thing:
>>>     - separate TLAB usage to contiguous allocations in eden space.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8191985
>>> WebRev: http://cr.openjdk.java.net/~rasbold/8191985/webrev.03/
>>>
>>> Does anyone see any issues with this webrev? Could someone test/verify
>>> it please?
>>>
>>> Thanks for your help,
>>> Jc
>>>
>>
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8191985: Tlab refills for ARM

Edward Nevill-5
Hi Robin,

I have eyeballed this, comparing it against the x86 version, and it looks good to me.

I'll try to build and test this, but it will probably be sometime between Christmas and New Year.

Just to confirm, the repo I should be patching and building is the following?

http://hg.openjdk.java.net/jdk/jdk

Thanks,
Ed.

On Wed, 2017-12-20 at 08:58 +0100, Robbin Ehn wrote:
> Hi JC,
>
> Looks good to me.
>
> Can someone else please eyeball this?
>
> /Robbin
>
On 2017-12-20 05:39, JC Beyler wrote:

>
> Hi all,
>
> Thanks Robbin for the eye-balling.
>
> I did update the webrev to here:
> http://cr.openjdk.java.net/~jcbeyler/8191985/webrev.01/
>
> Let me know what you think,
> Jc
>
> > > >
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8191985: Tlab refills for ARM

Dmitry Samersoff-3
In reply to this post by JC Beyler
JC Beyler,

Looks good to me.

But I didn't test it.

-Dmitry

On 12/20/2017 07:39 AM, JC Beyler wrote:

> Hi all,
>
> Thanks Robbin for the eye-balling.
>
> I did update the webrev to here:
> http://cr.openjdk.java.net/~jcbeyler/8191985/webrev.01/
>
> Let me know what you think,
> Jc
>
> On Tue, Dec 19, 2017 at 2:24 AM, Robbin Ehn <[hidden email]> wrote:
>> Hi JC,
>>
>> We do not longer have the ability to build and test arm32. And none seem to
>> be picking this port up.
>> Only eyeballing this to looks good!
>>
>> /Robbin
>>
>>
>> On 2017-12-14 17:08, 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 Arm is the same as for
>>> Sparc/X86/Aarch64 and is to do one thing:
>>>    - separate TLAB usage to contiguous allocations in eden space.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8191985
>>> WebRev: http://cr.openjdk.java.net/~rasbold/8191985/webrev.03/
>>>
>>> Does anyone see any issues with this webrev? Could someone test/verify
>>> it please?
>>>
>>> Thanks for your help,
>>> Jc
>>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR 8191985: Tlab refills for ARM

Daniel D. Daugherty
In reply to this post by Edward Nevill-5
On 12/20/17 8:21 AM, Edward Nevill wrote:
> Hi Robin,
>
> I have eyeballed this, comparing it against the x86 version, and it looks good to me.
>
> I'll try to build and test this, but it will probably be sometime between Christmas and New Year.
>
> Just to confirm, the repo I should be patching and building is the following?
>
> http://hg.openjdk.java.net/jdk/jdk

The webrev shows this line:

 > Compare against:    http://hg.openjdk.java.net/jdk10/hs

but that repo is stale with the creation of:

http://hg.openjdk.java.net/jdk/jdk

and

http://hg.openjdk.java.net/jdk/hs

I recommend HotSpot changes be made relative to

http://hg.openjdk.java.net/jdk/hs

Dan

>
> Thanks,
> Ed.
>
> On Wed, 2017-12-20 at 08:58 +0100, Robbin Ehn wrote:
>> Hi JC,
>>
>> Looks good to me.
>>
>> Can someone else please eyeball this?
>>
>> /Robbin
>>
> On 2017-12-20 05:39, JC Beyler wrote:
>> Hi all,
>>
>> Thanks Robbin for the eye-balling.
>>
>> I did update the webrev to here:
>> http://cr.openjdk.java.net/~jcbeyler/8191985/webrev.01/
>>
>> Let me know what you think,
>> Jc
>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR 8191985: Tlab refills for ARM

JC Beyler
Hi all,

Thanks for taking a look at the webrev!

Here is the change relative to jdk/hs:
http://cr.openjdk.java.net/~jcbeyler/8191985/webrev.02/

Let me know what you think, hopefully I didn't make a mistake when
making the webrev :)
Jc


On Wed, Dec 20, 2017 at 5:50 AM, Daniel D. Daugherty
<[hidden email]> wrote:

> On 12/20/17 8:21 AM, Edward Nevill wrote:
>>
>> Hi Robin,
>>
>> I have eyeballed this, comparing it against the x86 version, and it looks
>> good to me.
>>
>> I'll try to build and test this, but it will probably be sometime between
>> Christmas and New Year.
>>
>> Just to confirm, the repo I should be patching and building is the
>> following?
>>
>> http://hg.openjdk.java.net/jdk/jdk
>
>
> The webrev shows this line:
>
>> Compare against:    http://hg.openjdk.java.net/jdk10/hs
>
> but that repo is stale with the creation of:
>
> http://hg.openjdk.java.net/jdk/jdk
>
> and
>
> http://hg.openjdk.java.net/jdk/hs
>
> I recommend HotSpot changes be made relative to
>
> http://hg.openjdk.java.net/jdk/hs
>
> Dan
>
>
>>
>> Thanks,
>> Ed.
>>
>> On Wed, 2017-12-20 at 08:58 +0100, Robbin Ehn wrote:
>>>
>>> Hi JC,
>>>
>>> Looks good to me.
>>>
>>> Can someone else please eyeball this?
>>>
>>> /Robbin
>>>
>> On 2017-12-20 05:39, JC Beyler wrote:
>>>
>>> Hi all,
>>>
>>> Thanks Robbin for the eye-balling.
>>>
>>> I did update the webrev to here:
>>> http://cr.openjdk.java.net/~jcbeyler/8191985/webrev.01/
>>>
>>> Let me know what you think,
>>> Jc
>>>
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8191985: Tlab refills for ARM

Edward Nevill-5
Hi,

I have built and tested this on a Samsung Chromebook (armv7l).

The tests I ran were jtreg hotspot tests patched vs original in three configurations, default gc (g1gc), parallelgc and serialgc. The results in all cases were identical before and after patching.

Default gc (g1gc): passed: 1,435; failed: 9; error: 116
ParallelGC: passed: 1,331; failed: 8; error: 112
SerialGC: passed: 1,326; failed: 7; error: 112

Patch looks good to me.

All the best,
Ed.

On Wed, 2017-12-20 at 09:06 -0800, JC Beyler wrote:

> Hi all,
>
> Thanks for taking a look at the webrev!
>
> Here is the change relative to jdk/hs:
> http://cr.openjdk.java.net/~jcbeyler/8191985/webrev.02/
>
> Let me know what you think, hopefully I didn't make a mistake when
> making the webrev :)
> Jc
>
>
> On Wed, Dec 20, 2017 at 5:50 AM, Daniel D. Daugherty
> <[hidden email]> wrote:
> > On 12/20/17 8:21 AM, Edward Nevill wrote:
> > >
> > > Hi Robin,
> > >
> > > I have eyeballed this, comparing it against the x86 version, and it looks
> > > good to me.
> > >
> > > I'll try to build and test this, but it will probably be sometime between
> > > Christmas and New Year.
> > >
> > > Just to confirm, the repo I should be patching and building is the
> > > following?
> > >
> > >

Reply | Threaded
Open this post in threaded view
|

Re: RFR 8191985: Tlab refills for ARM

JC Beyler
Perfect, thanks Edward for testing :)
Jc


On Thu, Dec 28, 2017 at 10:14 AM, Edward Nevill <[hidden email]> wrote:

> Hi,
>
> I have built and tested this on a Samsung Chromebook (armv7l).
>
> The tests I ran were jtreg hotspot tests patched vs original in three configurations, default gc (g1gc), parallelgc and serialgc. The results in all cases were identical before and after patching.
>
> Default gc (g1gc): passed: 1,435; failed: 9; error: 116
> ParallelGC: passed: 1,331; failed: 8; error: 112
> SerialGC: passed: 1,326; failed: 7; error: 112
>
> Patch looks good to me.
>
> All the best,
> Ed.
>
> On Wed, 2017-12-20 at 09:06 -0800, JC Beyler wrote:
>> Hi all,
>>
>> Thanks for taking a look at the webrev!
>>
>> Here is the change relative to jdk/hs:
>> http://cr.openjdk.java.net/~jcbeyler/8191985/webrev.02/
>>
>> Let me know what you think, hopefully I didn't make a mistake when
>> making the webrev :)
>> Jc
>>
>>
>> On Wed, Dec 20, 2017 at 5:50 AM, Daniel D. Daugherty
>> <[hidden email]> wrote:
>> > On 12/20/17 8:21 AM, Edward Nevill wrote:
>> > >
>> > > Hi Robin,
>> > >
>> > > I have eyeballed this, comparing it against the x86 version, and it looks
>> > > good to me.
>> > >
>> > > I'll try to build and test this, but it will probably be sometime between
>> > > Christmas and New Year.
>> > >
>> > > Just to confirm, the repo I should be patching and building is the
>> > > following?
>> > >
>> > >
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8191985: Tlab refills for ARM

Edward Nevill-5
Hi,

Can this change be pushed to jdk/hs now? Have we got a formal Reviewer?
I can push it if required.

http://cr.openjdk.java.net/~jcbeyler/8191985/webrev.02/

https://bugs.openjdk.java.net/browse/JDK-8191985

All the best,
Ed.

On Tue, 2018-01-02 at 09:19 -0800, JC Beyler wrote:

> Perfect, thanks Edward for testing :)
> Jc
>
>
> On Thu, Dec 28, 2017 at 10:14 AM, Edward Nevill <edward.nevill@gmail.
> com> wrote:
> > Hi,
> >
> > I have built and tested this on a Samsung Chromebook (armv7l).
> >
> > The tests I ran were jtreg hotspot tests patched vs original in
> > three configurations, default gc (g1gc), parallelgc and serialgc.
> > The results in all cases were identical before and after patching.
> >
> > Default gc (g1gc): passed: 1,435; failed: 9; error: 116
> > ParallelGC: passed: 1,331; failed: 8; error: 112
> > SerialGC: passed: 1,326; failed: 7; error: 112
> >
> > Patch looks good to me.
> >
> > All the best,
> > Ed.
> >
> > On Wed, 2017-12-20 at 09:06 -0800, JC Beyler wrote:
> > > Hi all,
> > >
> > > Thanks for taking a look at the webrev!
> > >
> > > Here is the change relative to jdk/hs:
> > > http://cr.openjdk.java.net/~jcbeyler/8191985/webrev.02/
> > >
> > > Let me know what you think, hopefully I didn't make a mistake
> > > when
> > > making the webrev :)
> > > Jc
> > >
> > >
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8191985: Tlab refills for ARM

Andrew Haley
On 05/01/18 11:13, Edward Nevill wrote:
> Can this change be pushed to jdk/hs now? Have we got a formal Reviewer?
> I can push it if required.
>
> http://cr.openjdk.java.net/~jcbeyler/8191985/webrev.02/
>
> https://bugs.openjdk.java.net/browse/JDK-8191985

OK.

--
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8191985: Tlab refills for ARM

JC Beyler
Thanks both,

Anything I need to do on my side?
Jc

On Fri, Jan 5, 2018 at 9:15 AM, Andrew Haley <[hidden email]> wrote:

> On 05/01/18 11:13, Edward Nevill wrote:
>> Can this change be pushed to jdk/hs now? Have we got a formal Reviewer?
>> I can push it if required.
>>
>> http://cr.openjdk.java.net/~jcbeyler/8191985/webrev.02/
>>
>> https://bugs.openjdk.java.net/browse/JDK-8191985
>
> OK.
>
> --
> Andrew Haley
> Java Platform Lead Engineer
> Red Hat UK Ltd. <https://www.redhat.com>
> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8191985: Tlab refills for ARM

Dmitry Samersoff-3
In reply to this post by Edward Nevill-5
Edward,

On 05.01.2018 14:13, Edward Nevill wrote:
> Hi,
>
> Can this change be pushed to jdk/hs now?

> Have we got a formal Reviewer?

The fix looks good to me.

> I can push it if required.

Let me know if you need a help to push the fix.

-Dmitry

>
> http://cr.openjdk.java.net/~jcbeyler/8191985/webrev.02/
>
> https://bugs.openjdk.java.net/browse/JDK-8191985
>
> All the best,
> Ed.
>
> On Tue, 2018-01-02 at 09:19 -0800, JC Beyler wrote:
>> Perfect, thanks Edward for testing :)
>> Jc
>>
>>
>> On Thu, Dec 28, 2017 at 10:14 AM, Edward Nevill <edward.nevill@gmail.
>> com> wrote:
>>> Hi,
>>>
>>> I have built and tested this on a Samsung Chromebook (armv7l).
>>>
>>> The tests I ran were jtreg hotspot tests patched vs original in
>>> three configurations, default gc (g1gc), parallelgc and serialgc.
>>> The results in all cases were identical before and after patching.
>>>
>>> Default gc (g1gc): passed: 1,435; failed: 9; error: 116
>>> ParallelGC: passed: 1,331; failed: 8; error: 112
>>> SerialGC: passed: 1,326; failed: 7; error: 112
>>>
>>> Patch looks good to me.
>>>
>>> All the best,
>>> Ed.
>>>
>>> On Wed, 2017-12-20 at 09:06 -0800, JC Beyler wrote:
>>>> Hi all,
>>>>
>>>> Thanks for taking a look at the webrev!
>>>>
>>>> Here is the change relative to jdk/hs:
>>>> http://cr.openjdk.java.net/~jcbeyler/8191985/webrev.02/
>>>>
>>>> Let me know what you think, hopefully I didn't make a mistake
>>>> when
>>>> making the webrev :)
>>>> Jc
>>>>
>>>>