Re: [10] RFR: 8193507: [REDO] Startup regression due to JDK-8185582

[Xueming Shen]

On 12/15/17, 3:43 PM, Peter Levart wrote:

Hi Claes,

I see this is already pushed. I don't have any additional comments, 
but just want to know what was wrong with old code. You say that you 
used non-static inner classes. I don't see that in old code. All the 
lambdas you replaced with nested static classes should have already 
captured just local variables. I must be missing something and I would 
really like to know what.


Perter,

Since that code triggered all zip/cleaner regression tests failed. I 
rollback-ed that
fix overnight to make our overnight tests happy. So what in the webrev 
is against my

original code.


Here is my webrev that undo-ed the non-static inner classes.

http://cr.openjdk.java.net/~sherman/8193490/webrev/

Sherman



Regards, Peter

Claes Redestad je 14. 12. 2017 ob 12:07 napisal:

Hi,

my previous fix failed due to use of non-static inner classes which 
kept the cleanable objects around. Also, Sherman suggested a more 
thorough fix to Inflater/Deflater after I had already pushed.


Webrev: http://cr.openjdk.java.net/~redestad/8193507/open.00/

Verified all java/util/zip tests pass this time.

/Claes

Re: [10] RFR: 8193507: [REDO] Startup regression due to JDK-8185582

[Peter Levart]

Hi Claes,

I see this is already pushed. I don't have any additional comments, but 
just want to know what was wrong with old code. You say that you used 
non-static inner classes. I don't see that in old code. All the lambdas 
you replaced with nested static classes should have already captured 
just local variables. I must be missing something and I would really 
like to know what.


Regards, Peter

Claes Redestad je 14. 12. 2017 ob 12:07 napisal:

Hi,

my previous fix failed due to use of non-static inner classes which 
kept the cleanable objects around. Also, Sherman suggested a more 
thorough fix to Inflater/Deflater after I had already pushed.


Webrev: http://cr.openjdk.java.net/~redestad/8193507/open.00/

Verified all java/util/zip tests pass this time.

/Claes

Re: [11] RFR 8193085 Vectorize the nio Buffer equals and compareTo implementations

[Paul Sandoz]

> On 15 Dec 2017, at 13:01, David Lloyd  wrote:
> 
> I'm not a reviewer, but I was curious about this change; unfortunately
> the diff seems to be dominated by case and formatting changes making
> the actual functional aspect change hard to divine.
> 

If not already i recommend viewing via udiffs, i find that makes it easier.


> Within the JBoss unit we have an informal policy that formatting
> changes should be presented separately so that it's easier to trace
> back problems in the future, as well as being much easier to review
> the change in the first place.  Would I be stepping out of bounds to
> suggest that this change should be similarly divided?
> 

In hindsight :-) at this point i would prefer not to split it unless reviewers 
are having a really hard time. I was furiously hacking on this and got fed up 
with the names making it harder for me to reason about the code so i changed 
‘em mid-flight when doing this work.

Thanks,
Paul.

Re: [11] RFR 8193085 Vectorize the nio Buffer equals and compareTo implementations

[David Lloyd]

I'm not a reviewer, but I was curious about this change; unfortunately
the diff seems to be dominated by case and formatting changes making
the actual functional aspect change hard to divine.

Within the JBoss unit we have an informal policy that formatting
changes should be presented separately so that it's easier to trace
back problems in the future, as well as being much easier to review
the change in the first place.  Would I be stepping out of bounds to
suggest that this change should be similarly divided?

On Fri, Dec 15, 2017 at 2:29 PM, Paul Sandoz  wrote:
> Hi,
>
> Please review this patch to vectorize the buffer equals and compareTo 
> implementations, using the same approach that was used for arrays:
>
>   
> http://cr.openjdk.java.net/~psandoz/jdk/JDK-8193085-buffer-equals-compareTo-vectorize/webrev/
>  
> 
>
> This patch expands on using the double address mode of unsafe to uniformly 
> access a memory region covered by a buffer be it on or off heap. Only buffers 
> with the same endianness can support optimized equality and comparison.
>
> —
>
> A follow on issue will explore a mismatch method (thus enabling support for 
> float/double comparison that is compatible with arrays), and equals/compareTo 
> methods that accept absolute positions (to avoid the dup.pos.limit.slice 
> dance).
>
> Thanks,
> Paul.



-- 
- DML

[11] RFR 8193085 Vectorize the nio Buffer equals and compareTo implementations

[Paul Sandoz]

Hi,

Please review this patch to vectorize the buffer equals and compareTo 
implementations, using the same approach that was used for arrays:

  
http://cr.openjdk.java.net/~psandoz/jdk/JDK-8193085-buffer-equals-compareTo-vectorize/webrev/
 


This patch expands on using the double address mode of unsafe to uniformly 
access a memory region covered by a buffer be it on or off heap. Only buffers 
with the same endianness can support optimized equality and comparison.

—

A follow on issue will explore a mismatch method (thus enabling support for 
float/double comparison that is compatible with arrays), and equals/compareTo 
methods that accept absolute positions (to avoid the dup.pos.limit.slice dance).

Thanks,
Paul.

Re: RFR JDK-8193479: test/hotspot/jtreg/compiler/codegen/Test6896617.java fails after JDK-8184947

[Tobias Hartmann]

Hi Sherman,

On 15.12.2017 10:48, Tobias Hartmann wrote:
> I would say it's best to wait for the jdk10 repo to open.

I've checked with Jesper and we can/should quarantine this test right away. 
I'll take care of it [1].

Best regards,
Tobias

[1] 
http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2017-December/027933.html

Re: RFR JDK-8193479: test/hotspot/jtreg/compiler/codegen/Test6896617.java fails after JDK-8184947

[Tobias Hartmann]

Hi Sherman,

On 14.12.2017 21:46, Xueming Shen wrote:
> I have a webrev out there. But I will probably have to wait for the
> new jdk10 repo open next week.
> 
> Or maybe the hs repo is still open for something like this? and
> you guys can help get it in directly there?

I would say it's best to wait for the jdk10 repo to open.

> It appears the consensus is to put this one into the ProblemList and let the
> hotspot folks to fix the test case.
> 
> issue: https://bugs.openjdk.java.net/browse/JDK-8193479
> webrev: http://cr.openjdk.java.net/~sherman/8193479/webrev
> 
> The corresponding hotspot/compiler issue filed: JDK-8193549.

The correct way is to create a subtask to quarantine the test. I've created one 
for you and closed (JDK-8193549) as
duplicate:
https://bugs.openjdk.java.net/browse/JDK-8193608

Please un-assign 8193479 if you don't plan to fix the test.

Thanks,
Tobias