RFR(xs): 8247252: TestCombinedCompressedFlags.java failed src/hotspot/share/services/virtualMemoryTracker.cpp:388 Error: ShouldNotReachHere()
Thomas Stüfe
thomas.stuefe at gmail.com
Wed Jun 10 18:44:03 UTC 2020
Hi Yumin,
On Wed, Jun 10, 2020 at 7:42 PM Yumin Qi <yumin.qi at oracle.com> wrote:
> Hi, Thomas
>
>
> Ignore my comment --- use_requested_addr is not address. Sorry for
> this. BTW, I don't think you should change test cases to add NMT
> tracking, that will give too much information on output. I agree with
> Calvin you can always using vmoptions + group selection on jtreg for
> testing your change.
>
>
>
Thank you for the quick review!
About NMT tracking, unless I am mistaken you would see no output of NMT,
since NMT is never reported. We only collect the data.
The reason I added NMT tracking is because it is intricately interwoven
with CDS and metaspace initialization, and can easily break.
Thanks, Thomas
Thanks
>
> Yumin
>
> On 6/10/20 10:11 AM, Yumin Qi wrote:
> > Hi, Thomas
> >
> > Â Â I verified your patch, it looks good to me. One minor comment:
> >
> > 2271 result = MAP_ARCHIVE_MMAP_FAILURE;
> > 2272 log_debug(cds)("Failed to reserve spaces
> > (use_requested_addr=%u)", (unsigned)use_requested_addr); It is not
> > intuitive for an address printed as unsigned, could you using
> > PTR_FORMAT? Thanks Yumin
> >
> > On 6/10/20 6:09 AM, Thomas Stüfe 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
>
More information about the hotspot-runtime-dev
mailing list