RFR (S) 8215724: Epsilon: ArrayStoreExceptionTest.java fails; missing arraycopy check
leo.korinth at oracle.com
Wed Jan 9 09:31:30 UTC 2019
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:
Looks good to me!
> webrev.01 passed jdk-submit, and there are no code changes in webrev.02, only fixed comment.
More information about the hotspot-gc-dev