Hi Stefan, Both changes look good to me!
Thanks, Yasumasa 2019年2月15日(金) 2:12 Stefan Karlsson <stefan.karls...@oracle.com>: > > Hi again, > > I've separated the live regions iteration refactoring into this patch: > https://cr.openjdk.java.net/~stefank/8219003/webrev.01/ > > And use this RFE for the ZGC specific parts: > https://cr.openjdk.java.net/~stefank/8218922/webrev.02/ > > Thanks, > StefanK > > On 2019-02-14 14:39, Stefan Karlsson wrote: > > Hi Yasumasa, > > > > On 2019-02-14 14:11, Yasumasa Suenaga wrote: > >> Hi Stefan, > >> > >>> Maybe this is enough to enable a bit more SA debugging capabilities > >>> when > >>> running with ZGC? What do you think, should we bring in this change? > >> > >> I think it should be brought this in. > >> I filed same issue as JDK-8207843, but I've not yet worked. > >> So I will close it as duplicate of your change. > >> > >> > >>> To be able to implement this more cleanly I've restructured the live > >>> region collection, and pushed GC specific code into the specific GCs. > >>> There are some extra usage of generics to make the code a bit easier to > >>> read and develop. > >> > >> IMHO refactoring for live region collection and ZGC related changes > >> should > >> be separated. What do you think? > > > > Yes. I think that would be good. I'll separate this out into two changes. > > > >> > >> > >> Your change looks good to me. > >> BTW, did you check `jhsdb jmap --binaryheap` with this change? > >> > > > > Yes, when testing this I ran all tests in serviceability/sa and > > manually tested the command above. > > > > While testing this I also had this patch applied to enable SA hprof > > for ZGC: > > http://cr.openjdk.java.net/~stefank/8218970/webrev.01/ > > > > And used this patch to turn off the ZGC filtering in the tests: > > http://cr.openjdk.java.net/~stefank/8218978/webrev.01/ > > > > I'm currently rerunning the tests to see that the latest changes > > didn't break anything. > > > > Thanks, > > StefanK > >> > >> Thanks, > >> > >> Yasumasa > >> > >> > >> > >> On 2019/02/13 23:52, Stefan Karlsson wrote: > >>> Hi all, > >>> > >>> Please review / comment on this patch to enable a best-effort live heap > >>> region iteration implementation in ZGC. > >>> > >>> http://cr.openjdk.java.net/~stefank/8218922/webrev.01/ > >>> https://bugs.openjdk.java.net/browse/JDK-8218922 > >>> > >>> The SA has functionally that relies on live heap region information > >>> from > >>> the GCs. This is problematic when dead objects are left in the heap, > >>> and > >>> their classes have been unloaded. > >>> > >>> Because of this ZGC has so far not implemented this feature. > >>> However, we > >>> could provide a best-effort implementation that most likely will > >>> work if > >>> classes are not unloaded (or class unloading is turned off), and > >>> otherwise might fail to fully parse and report all live heap regions. > >>> > >>> For active, non-relocating pages the patch simply returns [start, top) > >>> and for pages being actively relocated, it reports regions containing > >>> the non-forwarded objects, the other objects are either dead or > >>> could be > >>> found in one of the to-regions. > >>> > >>> With this patch I'm able to get heap histograms with ZGC. > >>> > >>> Maybe this is enough to enable a bit more SA debugging capabilities > >>> when > >>> running with ZGC? What do you think, should we bring in this change? > >>> > >>> To be able to implement this more cleanly I've restructured the live > >>> region collection, and pushed GC specific code into the specific GCs. > >>> There are some extra usage of generics to make the code a bit easier to > >>> read and develop. > >>> > >>> Thanks, > >>> StefanK > >>> >
