RFR(xs): 8247252: TestCombinedCompressedFlags.java failed src/hotspot/share/services/virtualMemoryTracker.cpp:388 Error: ShouldNotReachHere()

[Thomas Stüfe]

May I have a second review please?

Thanks!

..Thomas

On Wed, Jun 10, 2020 at 3:09 PM Thomas Stüfe <thomas.stuefe at gmail.com>
wrote:

> Hi all,
>
> could I please have reviews for this bug fix.
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8247252
> webrev:
> http://cr.openjdk.java.net/~stuefe/webrevs/8247252-cds-nmt-ccps-off/webrev.00/webrev/
>
> This bug was introduced with "8243535 NMT may show wrong numbers for CDS
> and CCS".
>
> The error is that a VM loading a shared archive but having compressed
> class pointers disabled would reserve space for the CDS but fail to
> register it with the correct tag with NMT. So, inside NMT it still is
> marked as "mtUnknown". This leads to a follow up error when the actual
> archive files are to be mapped into this reserved region, since the region
> is expected to carry the "mtClassShared" tag for this to work.
>
> The fix is simple to add the missing registration in metaspaceShared.cpp
> when the cds has been reserved for the non-class-space case:
>
> @@ -2469,10 +2470,12 @@
>      archive_space_rs = ReservedSpace(archive_space_size,
> archive_space_alignment,
>                                       false /* bool large */,
> (char*)base_address);
>      if (archive_space_rs.is_reserved()) {
>        assert(base_address == NULL ||
>               (address)archive_space_rs.base() == base_address, "Sanity");
> +      // Register archive space with NMT.
> +      MemTracker::record_virtual_memory_type(archive_space_rs.base(),
> mtClassShared);
>        return archive_space_rs.base();
>      }
>      return NULL;
>    }
>
> All the rest is fluff:
>
> - added logging in a number of places to make analysis of these kinds of
> errors simpler
> - explicitly switched on NMT for a number of cds jtreg tests.
>
> ---
>
> The reason we did not see these errors before in our (SAPs) CI was that we
> run our tests with NMT (by default) switched off. Apparently Oracle runs
> the jtreg tests with NMT enabled via -vmoptions? We will change our CI to
> do this too. But it would be good if the test flags would be published
> somewhere, and we would all use the same flags.
>
> Thanks, Thomas
>