RFR 8234270: [REDO] JDK-8204128 NMT might report incorrect numbers for Compiler area
Zhengyu Gu
zgu at redhat.com
Tue Nov 26 12:59:55 UTC 2019
Hi Thomas,
>
> Right, we certainly can.
>
> Updated webrev: http://cr.openjdk.java.net/~zgu/JDK-8234270/webrev.01/
>
>
> output.shouldContain("Test (reserved=2GB, committed=2GB)");
>
> Does this work? Does the output round sufficiently to always show "2G"
> even though the total can jitter by +- 10M?
NMT rounds it to nearest whole number, so 2GB +/- 10M won't make a dent.
>
> New patch also passed submit test:
> [Mach5] mach5-one-zgu-JDK-8234270-2-20191126-0012-6997789: PASSED
>
>
> Some more remarks and questions:
>
> http://cr.openjdk.java.net/~zgu/JDK-8234270/webrev.01/src/hotspot/share/memory/resourceArea.cpp.udiff.html
> @@ -31,10 +31,13 @@
>
> void ResourceArea::bias_to(MEMFLAGS new_flags) {
> if (new_flags != _flags) {
> MemTracker::record_arena_free(_flags);
> MemTracker::record_new_arena(new_flags);
> + size_t size = size_in_bytes();
> + MemTracker::record_arena_size_change(-ssize_t(size), _flags); (A)
> + MemTracker::record_arena_size_change(ssize_t(size), new_flags);
> _flags = new_flags;
> }
>
> Just aesthetics, but coding would be easier to understand if you
> reordered things:
>
> if (new_flags != _flags) {
> + size_t size = size_in_bytes();
> + MemTracker::record_arena_size_change(-ssize_t(size), _flags); (A)
> MemTracker::record_arena_free(_flags);
> MemTracker::record_new_arena(new_flags);
> + MemTracker::record_arena_size_change(ssize_t(size), new_flags);
> _flags = new_flags;
> }
>
> or, if you were extending record_arena_free/record_new_arena to take
> last/initial arena size too and pass that on to
> MallocMemory::deallocate()/allocate().
>
> But I leave it up tp you if you change this. If you just reorder the
> calls, I do not need another Webrev,
I will re-order the calls, as you suggested.
Thanks,
-Zhengyu
>
> ...
>
> Another unrelated question, what is the reason for the unusual creation
> of MallocMemorySummarySnapshot with placement new? Why not just put it
> as a member into MallocMemorySummary? I must be missing something.
>
>
> -Zhengyu
>
>
>
> >
> > 66 wb.NMTFreeArena(arena1);
> >
> > On Mon, Nov 25, 2019 at 2:30 PM Zhengyu Gu <zgu at redhat.com
> <mailto:zgu at redhat.com>
> > <mailto:zgu at redhat.com <mailto:zgu at redhat.com>>> wrote:
> >
> > Ping ... May I get a second review?
> >
> > Thanks,
> >
> > -Zhengyu
> >
> > On 11/21/19 12:12 PM, yumin qi wrote:
> > > Hi, Zhengyu
> > >
> > > The fix looks good to me.
> > >
> > > Thanks
> > > Yumin
> > >
> > >
> > >
> > > On Wed, Nov 20, 2019 at 5:49 AM Zhengyu Gu <zgu at redhat.com
> <mailto:zgu at redhat.com>
> > <mailto:zgu at redhat.com <mailto:zgu at redhat.com>>
> > > <mailto:zgu at redhat.com <mailto:zgu at redhat.com>
> <mailto:zgu at redhat.com <mailto:zgu at redhat.com>>>> wrote:
> > >
> > > JDK-8204128 did not fix the original bug. But new
> assertion
> > helped to
> > > catch the problem, as it consistently failed in Oracle
> > internal tests.
> > >
> > > The root cause is that, when NMT biases a resource area to
> > compiler, it
> > > did not adjust tracking data to reflect that. When the
> biased
> > resource
> > > area is released, there is a possibility that its size is
> > greater than
> > > total size recorded, and underflow a size_t counter.
> > >
> > > JDK-8204128 patch also missed a long to ssize_t parameter
> > type change,
> > > that resulted new test failure on Windows, because long is
> > 4-bytes on
> > > Windows.
> > >
> > > Many thanks to Leonid Mesnik, who helped to run this patch
> > through
> > > Oracle's internal stress tests.
> > >
> > > Bug: https://bugs.openjdk.java.net/browse/JDK-8234270
> > > Webrev:
> > http://cr.openjdk.java.net/~zgu/JDK-8234270/webrev.00/index.html
> > >
> > >
> > > Test:
> > > hotspot_nmt
> > > Submit test
> > > Oracle internal stress tests.
> > >
> > >
> > > Thanks,
> > >
> > > -Zhengyu
> > >
> >
>
More information about the hotspot-runtime-dev
mailing list