Dear all,
May I ask you help to review? This RFR has been there for quite a while.
Thanks!
BRs,
Lin
> On 2020/3/16, 5:18 PM, "linzang(臧琳)" <linz...@tencent.com> wrote:>
> Just update a new path, my preliminary measure show about 3.5x speedup of
> jmap histo on a nearly full 4GB G1 heap (8-core platform with parallel thread
> number set to 4).
> webrev: http://cr.openjdk.java.net/~lzang/jmap-8214535/8215624/webrev_02/
> bug: https://bugs.openjdk.java.net/browse/JDK-8215624
> CSR: https://bugs.openjdk.java.net/browse/JDK-8239290
> BRs,
> Lin
> > On 2020/3/2, 9:56 PM, "linzang(臧琳)" <linz...@tencent.com> wrote:
> >
> > Dear all,
> > Let me try to ease the reviewing work by some explanation :P
> > The patch's target is to speed up jmap -histo for heap
> iteration, from my experience it is necessary for large heap investigation.
> E.g in bigData scenario I have tried to conduct jmap -histo against 180GB
> heap, it does take quite a while.
> > And if my understanding is corrent, even the jmap -histo
> without "live" option does heap inspection with heap lock acquired. so it is
> very likely to block mutator thread in allocation-sensitive scenario. I would
> say the faster the heap inspection does, the shorter the mutator be blocked.
> This is parallel iteration for jmap is necessary.
> > I think the parallel heap inspection should be applied to all
> kind of heap. However, consider the heap layout are different for GCs, much
> time is required to understand all kinds of the heap layout to make the whole
> change. IMO, It is not wise to have a huge patch for the whole solution at
> once, and it is even harder to review it. So I plan to implement it
> incrementally, the first patch (this one) is going to confirm the
> implemention detail of how jmap accept the new option, passes it to
> attachListener of the jvm process and then how to make the parallel
> inspection closure be generic enough to make it easy to extend to different
> heap layout. And also how to implement the heap inspection in specific gc's
> heap. This patch use G1's heap as the begining.
> > This patch actually do several things:
> > 1. Add an option "parallelThreadNum=<N>" to jmap -histo, the
> default behavior is to set N to 0, means let's JVM decide how many threads to
> use for heap inspection. Set this option to 1 will disable parallel heap
> inspection. (more details in CSR:
> https://bugs.openjdk.java.net/browse/JDK-8239290)
> > 2. Make a change in how Jmap passing arguments, changes in
> http://cr.openjdk.java.net/~lzang/jmap-8214535/8215624/webrev_01/src/jdk.jcmd/share/classes/sun/tools/jmap/JMap.java.udiff.html,
> originally it pass options as separate arguments to attachListener, this
> patch change to that all options be compose to a single string. So the
> arg_count_max in attachListener.hpp do not need to be changed, and hence
> avoid the compatibility issue, as disscussed at
> https://mail.openjdk.java.net/pipermail/serviceability-dev/2019-March/027334.html
> > 3. Add an abstract class ParHeapInspectTask in
> heapInspection.hpp / heapInspection.cpp, It's work(uint worker_id) method
> prepares the data structure (KlassInfoTable) need for every parallel worker
> thread, and then call do_object_iterate_parallel() which is heap specific
> implementation. I also added some machenism in KlassInfoTable to support
> parallel iteration, such as merge().
> > 4. In specific heap (G1 in this patch), create a subclass of
> ParHeapInspectTask, implement the do_object_iterate_parallel() for parallel
> heap inspection. For G1, it simply invoke g1CollectedHeap's
> object_iterate_parallel().
> > 5. Add related test.
> > 6. it may be easy to extend this patch for other kinds of heap
> by creating subclass of ParHeapInspectTask and implement the
> do_object_iterate_parallel().
> >
> > Hope these info could help on code review and initate the
> discussion :-)
> > Thanks!
> >
> > BRs,
> > Lin
> > >On 2020/2/19, 9:40 AM, "linzang(臧琳)" <linz...@tencent.com> wrote:.
> > >
> > > Re-post this RFR with correct enhancement number to make it
> trackable.
> > > please ignore the previous wrong post. sorry for troubles.
> > >
> > > webrev:
> http://cr.openjdk.java.net/~lzang/jmap-8214535/8215624/webrev_01/
> > > Hi bug: https://bugs.openjdk.java.net/browse/JDK-8215624
> > > CSR: https://bugs.openjdk.java.net/browse/JDK-8239290
> > > --------------
> > > Lin
> > > >Hi Lin,
> > > >
> > > >Could you, please, re-post your RFR with the right
> enhancement number in
> > > >the message subject?
> > > >It will be more trackable this way.
> > > >
> > > >Thanks,
> > > >Serguei
> > > >
> > > >
> > > >On 2/17/20 10:29 PM, linzang(臧琳) wrote:
> > > >> Dear David,
> > > >> Thanks a lot!
> > > >> I have updated the refined code to
> http://cr.openjdk.java.net/~lzang/jmap-8214535/8215264/webrev_01/.
> > > >> IMHO the parallel heap inspection can be extended to
> all kinds of heap as long as the heap layout can support parallel iteration.
> > > >> Maybe we can firstly use this webrev to discuss how
> to implement it, because I am not sure my current implementation is an
> appropriate way to communicate with collectedHeap, then we can extend the
> solution to other kinds of heap.
> > > >>
> > > >> Thanks,
> > > >> --------------
> > > >> Lin
> > > >>> Hi Lin,
> > > >>>
> > > >>> Adding in hotspot-gc-dev as they need to see how this
> interacts with GC
> > > >>> worker threads, and whether it needs to be extended beyond
> G1.
> > > >>>
> > > >>> I happened to spot one nit when browsing:
> > > >>>
> > > >>> src/hotspot/share/gc/shared/collectedHeap.hpp
> > > >>>
> > > >>> + virtual bool run_par_heap_inspect_task(KlassInfoTable*
> cit,
> > > >>> +
> BoolObjectClosure* filter,
> > > >>> + size_t*
> missed_count,
> > > >>> + size_t
> thread_num) {
> > > >>> + return NULL;
> > > >>>
> > > >>> s/NULL/false/
> > > >>>
> > > >>> Cheers,
> > > >>> David
> > > >>>
> > > >>> On 18/02/2020 2:15 pm, linzang(臧琳) wrote:
> > > >>>> Dear All,
> > > >>>> May I ask your help to review the follow changes:
> > > >>>> webrev:
> > > >>>>
> http://cr.openjdk.java.net/~lzang/jmap-8214535/8215264/webrev_00/
> > > >>>> bug: https://bugs.openjdk.java.net/browse/JDK-8215624
> > > >>>> related CSR:
> https://bugs.openjdk.java.net/browse/JDK-8239290
> > > >>>> This patch enable parallel heap inspection of G1
> for jmap histo.
> > > >>>> my simple test shown it can speed up 2x of jmap
> -histo with
> > > >>>> parallelThreadNum set to 2 for heap at ~500M on 4-core
> platform.
> > > >>>>
> > > >>>>
> ------------------------------------------------------------------------
> > > >>>> BRs,
> > > >>>> Lin
> > > >> >
> > > >
