On Wed, 30 Sep 2020 21:28:19 GMT, Ziviani
<[email protected]> wrote:
>> TestInstanceKlassSize was failing because, for PowerPC, the following code
>> (instanceKlass.cpp) always compiles to
>> `return false;` bool InstanceKlass::has_stored_fingerprint() const {
>> #if INCLUDE_AOT
>> return should_store_fingerprint() || is_shared();
>> #else
>> return false;
>> #endif
>> }
>> However, in `hasStoredFingerprint()@InstanceKlass.java` the condition
>> `shouldStoreFingerprint() || isShared();` is
>> always evaluated and may return true (_AFAIK isShared() returns true_). Such
>> condition adds 8 bytes in the
>> `getSize()@InstanceKlass.java` causing the failure in TestInstanceKlassSize:
>> public long getSize() { // in number of
>> bytes
>> ...
>> if (hasStoredFingerprint()) {
>> size += 8; // uint64_t
>> }
>> return alignSize(size);
>> }
>> Considering these tests are failing for PowerPC only (_based on
>> ProblemList.txt_), my solution checks if
>> `hasStoredFingerprint()` is running on a PowerPC platform. I decided to go
>> this way because there is no existing flag
>> informing whether AOT is included or not and creating a new one just to
>> handle the PowerPC case seems too much. This
>> patch is an attempt to fix https://bugs.openjdk.java.net/browse/JDK-8230664
>
> Ziviani has updated the pull request with a new target base due to a merge or
> a rebase. The incremental webrev excludes
> the unrelated changes brought in by the merge/rebase. The pull request
> contains one additional commit since the last
> revision:
> 8230664: Fix TestInstanceKlassSize
>
> The code hasStoredFingerprint() at InstanceKlass.java is not considering
> AOT disabled at compilation time, like has_stored_fingerprint() at
> instanceKlass.cpp does. Such difference can cause TestInstanceKlassSize
> failures because all objects will have an extra 8-bytes.
LGTM
-------------
Marked as reviewed by sspitsyn (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/358
