RFR (S) 8215724: Epsilon: ArrayStoreExceptionTest.java fails; missing arraycopy check

[Leo Korinth]

Hi!

On 08/01/2019 19:48, Aleksey Shipilev wrote:
> On 1/8/19 6:30 PM, Leo Korinth wrote:
>> I guess you tried to use the already existing test (ArrayStoreExceptionTest.java) before you created
>> the new test. What was the reason for not using it in the end? I would rather like that you tried to
>> use the old test case, I believe it would improve overall testing and maintainability.
> 
> I prefer to have a targeted test that would test check-casted arraycopy for interpreter, C1 and C2
> barrier sets, see the @run clauses there. It is in line with other Epsilon tests. It is also
> specially crafted to work with Epsilon, e.g. it does not allocate all that much.
I see, having the old test separate will give more head room for 
allocation heavy changes in the old, common test cases. I do agree; that 
is more important than common test code.

> Plus it would run in tier1 without any additional testing options. ArrayStoreExceptionTest would
> require running with TEST_VM_OPTS or such.
> 
>> Also, there is a slight misleading comment on the #endif in barrierSet.inline.hpp, maybe instead use
>> the style #endif // pragma once ;-)
>>
>>    24 #ifndef SHARE_VM_GC_SHARED_BARRIERSET_INLINE_HPP
>>    25 #define SHARE_VM_GC_SHARED_BARRIERSET_INLINE_HPP
>>    60 #endif // SHARE_VM_GC_SHARED_BARRIERSET_HPP
> 
> I read the entire discussion about #pragma once (JDK-8216022), and it seems there is no consensus
> yet about using it, and also there are reports that some compilers people use are not supporting it.
> So, I fixed the comment instead. I hope Coleen has the script that does #pragma-once-ing, so it can
> catch the new file!

Thanks for updating the comment, I have not looked at Coleen's script, 
but I guess your change might help.

> Updated webrev:
>    http://cr.openjdk.java.net/~shade/8215724/webrev.02/
>

Looks good to me!

Thanks,
Leo

> webrev.01 passed jdk-submit, and there are no code changes in webrev.02, only fixed comment.
> 
> -Aleksey
>