JDK 9 rampdown and a plea for mercy

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

JDK 9 rampdown and a plea for mercy

Andrew Haley
As you may remember, I did some work on ByteBuffers a couple of years
ago.  At the time the generated code was sparklingly good, and I was
happy the job was done well.  Yesterday I looked at the code we are
generating in JDK9, and was horrified to see that it now is a steaming
pile of ordure.  It might be, of course, that I am mistaken about it
being better before, but I really don't think so: I wouldn't have let
it go out like that.  I suppose it was just good luck (or a C2 bug)
that great code was generated.

The full gory details are in
https://bugs.openjdk.java.net/browse/JDK-8176513, but the bottom line
is that unless this regression is fixed all my ByteBuffer work will
have been for naught.

Roland has had a look at what is happening, and he thinks that this
can be fixed fairly quickly: he already has a working patch.  Is there
any way that we can get it in?

One thing for the future: we need to keep an eye on code quality
regressions.  I'm not sure how, exactly.  And perhaps I need to
remember that just because C2 generates good code today, it might not
generate good code tomorrow.

Thanks,

Andrew.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: JDK 9 rampdown and a plea for mercy

Volker Simonis
On Fri, Mar 10, 2017 at 3:55 PM, Andrew Haley <[hidden email]> wrote:

> As you may remember, I did some work on ByteBuffers a couple of years
> ago.  At the time the generated code was sparklingly good, and I was
> happy the job was done well.  Yesterday I looked at the code we are
> generating in JDK9, and was horrified to see that it now is a steaming
> pile of ordure.  It might be, of course, that I am mistaken about it
> being better before, but I really don't think so: I wouldn't have let
> it go out like that.  I suppose it was just good luck (or a C2 bug)
> that great code was generated.
>
> The full gory details are in
> https://bugs.openjdk.java.net/browse/JDK-8176513, but the bottom line
> is that unless this regression is fixed all my ByteBuffer work will
> have been for naught.
>
> Roland has had a look at what is happening, and he thinks that this
> can be fixed fairly quickly: he already has a working patch.  Is there
> any way that we can get it in?
>
> One thing for the future: we need to keep an eye on code quality
> regressions.  I'm not sure how, exactly.  And perhaps I need to
> remember that just because C2 generates good code today, it might not
> generate good code tomorrow.
>

I totally agree. One possibility would be to expose the OptoAssembly
trough the WhiteBox API. That way we could write tests which ensure
that certain patterns are really matched in the expected way. It will
still be hard to write such tests because they will be mostly platform
specific, but at least it could be a start. One problem with the
OptoAssembly (i.e. the name of the corresponding nodes) is that they
aren't available in the opt/product builds. I think making them
available wouldn't be too much of an overhead and could be acceptable.
I once had a corresponding change, but never made it available.
"JDK-8046030: WhiteBox :: get native code disassembly or optoassembly"
[1] tracked a simiklar issue, but was closed :(

In our commercial VM we have some tests where we dump all the matched
nodes just to ensure that every expected node is still matched. But
that's of course just a start for checking that the generated code for
a method is good (and remains good):

Regards,
Volker

[1] https://bugs.openjdk.java.net/browse/JDK-8046030

> Thanks,
>
> Andrew.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RE: JDK 9 rampdown and a plea for mercy

Uwe Schindler-4
In reply to this post by Andrew Haley
Thanks Andrew,

I already sent a similar e-mail about Lucene to the jdk9-dev and hotspot-dev lists... I hope we can fix this! PLEASE, PLEASE! :-)

Uwe

-----
Uwe Schindler
[hidden email]
ASF Member, Apache Lucene PMC / Committer
Bremen, Germany
http://lucene.apache.org/

> -----Original Message-----
> From: hotspot-dev [mailto:[hidden email]] On
> Behalf Of Andrew Haley
> Sent: Friday, March 10, 2017 3:56 PM
> To: John Rose <[hidden email]>; Vladimir Kozlov
> <[hidden email]>
> Cc: hotspot-dev Source Developers <[hidden email]>
> Subject: JDK 9 rampdown and a plea for mercy
>
> As you may remember, I did some work on ByteBuffers a couple of years
> ago.  At the time the generated code was sparklingly good, and I was
> happy the job was done well.  Yesterday I looked at the code we are
> generating in JDK9, and was horrified to see that it now is a steaming
> pile of ordure.  It might be, of course, that I am mistaken about it
> being better before, but I really don't think so: I wouldn't have let
> it go out like that.  I suppose it was just good luck (or a C2 bug)
> that great code was generated.
>
> The full gory details are in
> https://bugs.openjdk.java.net/browse/JDK-8176513, but the bottom line
> is that unless this regression is fixed all my ByteBuffer work will
> have been for naught.
>
> Roland has had a look at what is happening, and he thinks that this
> can be fixed fairly quickly: he already has a working patch.  Is there
> any way that we can get it in?
>
> One thing for the future: we need to keep an eye on code quality
> regressions.  I'm not sure how, exactly.  And perhaps I need to
> remember that just because C2 generates good code today, it might not
> generate good code tomorrow.
>
> Thanks,
>
> Andrew.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: JDK 9 rampdown and a plea for mercy

Roland Westrelin-3
In reply to this post by Andrew Haley

> Roland has had a look at what is happening, and he thinks that this
> can be fixed fairly quickly: he already has a working patch.  Is there
> any way that we can get it in?

The 3 changes I think are needed are:

- we add MemBarCPUOrder around accesses when we don't know whether they
  are on heap or off heap (i.e. if base == null or not). That hurts
  optimizations a lot. I'd like to enable profiling of arguments at
  calls to unsafe methods so we can have speculative data on base and
  can then null check it if profiling tells us it's not null.

- We add MemBarCPUOrder around all "mismatched" accesses (integer write
  to byte array). That seems too strong to me for arrays. I'd like to
  relax that.

- There's an addressing shape that's not supported by superword that
  prevent vectorization from triggering with ByteBuffers. I think that
  can be fixed.

Roland.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: JDK 9 rampdown and a plea for mercy

Andrew Haley
On 10/03/17 19:54, Roland Westrelin wrote:

>
>> Roland has had a look at what is happening, and he thinks that this
>> can be fixed fairly quickly: he already has a working patch.  Is there
>> any way that we can get it in?
>
> The 3 changes I think are needed are:
>
> - we add MemBarCPUOrder around accesses when we don't know whether they
>   are on heap or off heap (i.e. if base == null or not). That hurts
>   optimizations a lot. I'd like to enable profiling of arguments at
>   calls to unsafe methods so we can have speculative data on base and
>   can then null check it if profiling tells us it's not null.
>
> - We add MemBarCPUOrder around all "mismatched" accesses (integer write
>   to byte array). That seems too strong to me for arrays. I'd like to
>   relax that.
>
> - There's an addressing shape that's not supported by superword that
>   prevent vectorization from triggering with ByteBuffers. I think that
>   can be fixed.

I think we should concentrate on whatever is the simplest and
least-potentially-breaking change.  MemBarCPUOrders wreck the
performance of most HeapByteBuffer writes so if we can get rid of them
we should, IMO.  I've certainly heard people say that ByteBuffers are
slow, and I really want that problem to go away.  Vectorization is the
cherry on top of the cake, and I can live without it.

Andrew.
Loading...