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 > > >> > > > >
