RFR: 8230664: Fix TestInstanceKlassSize for PowerPC
Ziviani
github.com+670087+jrziviani at openjdk.java.net
Tue Sep 29 20:59:05 UTC 2020
On Tue, 29 Sep 2020 20:03:06 GMT, Chris Plummer <cjplummer at openjdk.org> 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
>
> Can you please update the JBS issue to accurately describe what the underlying cause is. It incorreclty states that it
> is 8-byte vs 16-byte aligment.
> I'd prefer that you added someting like VM.hasAOT(). This will fix the problem for other CPU ports that may be in a
> similar situation and also ensure correctness when configure a build with `--disable-aot`.
@plummercj
Thank you for your review!
> Can you please update the JBS issue to accurately describe what the underlying cause is. It incorreclty states that it
> is 8-byte vs 16-byte aligment.
Unfortunately, I don't have write access to the bug system. I'll check if the original reporter would mind adding it
for me.
> I'd prefer that you added someting like VM.hasAOT(). This will fix the problem for other CPU ports that may be in a
> similar situation and also ensure correctness when configure a build with --disable-aot.
Sure, I'll work on it. I'll update this PR as soon as I finish the tests.
Thank you!
-------------
PR: https://git.openjdk.java.net/jdk/pull/358
More information about the serviceability-dev
mailing list