Looks good. /Staffan
On 19 aug 2013, at 11:03, Kevin Walls <kevin.wa...@oracle.com> wrote: > > Thanks Yumin. Just need a second opinion/review then... > > > > On 09/08/2013 16:03, Yumin Qi wrote: >> Looks good. Thanks for fixing it. >> >> Yumin >> >> On 8/9/2013 7:54 AM, Kevin Walls wrote: >>> Hi, >>> >>> Review request for a small change in ClassDump: >>> >>> http://cr.openjdk.java.net/~kevinw/8022655/webrev.00/ >>> >>> This was caused by moving some code from ClassDump's main method, breaking >>> the intent that it has to maintain either an output director, OR a >>> jarStream. Setting of the directory could be done unconditionally when it >>> was in main(), but not when in the run() method. >>> >>> The tests test/compiler/ciReplay for compiler replay work (if you update >>> sa.js with a suggested fix from 8011888). >>> >>> Trying those tests now on Solaris, I found coreadm settings may mean they >>> don't find the core: putting a coreadm command in the test means we should >>> find JTwork/scratch/core >>> >>> Thanks >>> Kevin >>> >>> >>> >>> On 08/08/2013 19:25, Yumin Qi wrote: >>>> Kevin, >>>> >>>> https://jbs.oracle.com/bugs/browse/JDK-8022655 >>>> >>>> I assigned this to you since it is caused by 8011888 fix. >>>> >>>> Thanks >>>> Yumin >>>> >>>> On 7/26/2013 2:20 AM, Kevin Walls wrote: >>>>> >>>>> Hi Yumin, >>>>> >>>>> Sure no problem I'll look into it. Looks like in ClassDump we should >>>>> check classFilter != null before doing that initialisation in the run() >>>>> method. >>>>> >>>>> At the moment trying jtreg on test/compiler/ciReplay I get a failure in >>>>> TestSA.sh, as it calls sun.jvm.hotspot.CLHSDB, and gives the javascript >>>>> exceptions characteristic of 8011888. Rebuilding with the suggested new >>>>> sa.js I see in that bug, I'll test and let you know... >>>>> >>>>> Thanks! >>>>> Kevin >>>>> >>>>> >>>>> >>>>> On 26/07/13 02:17, Yumin Qi wrote: >>>>>> Hi, Kevin >>>>>> >>>>>> http://cr.openjdk.java.net/~kevinw/8010278/webrev.03/ >>>>>> <http://cr.openjdk.java.net/%7Ekevinw/8010278/webrev.03/> >>>>>> I found this fix breaks C2 replay for dumping loaded java classes: >>>>>> >>>>>> at >>>>>> sun.jvm.hotspot.tools.jcore.PackageNameFilter.canInclude(PackageNameFilter.java:55) >>>>>> at >>>>>> sun.jvm.hotspot.tools.jcore.ClassDump.dumpKlass(ClassDump.java:135) >>>>>> at >>>>>> sun.jvm.hotspot.tools.jcore.ClassDump.access$000(ClassDump.java:38) >>>>>> at >>>>>> sun.jvm.hotspot.tools.jcore.ClassDump$1.visit(ClassDump.java:106) >>>>>> at sun.jvm.hotspot.memory.Dictionary.classesDo(Dictionary.java:68) >>>>>> at >>>>>> sun.jvm.hotspot.memory.SystemDictionary.classesDo(SystemDictionary.java:190) >>>>>> at sun.jvm.hotspot.tools.jcore.ClassDump.run(ClassDump.java:102) >>>>>> at >>>>>> sun.jvm.hotspot.DumpLoadedClasses.run(DumpLoadedClasses.java:65) >>>>>> at >>>>>> sun.jvm.hotspot.DumpLoadedClasses.main(DumpLoadedClasses.java:43) >>>>>> >>>>>> The problem is with Replay, we set BootFilter and NonBootFilter for >>>>>> ClassDump, but this changeset ignored the existing filter. >>>>>> I filed a bug against this problem. >>>>>> >>>>>> https://jbs.oracle.com/bugs/browse/JDK-8021444 >>>>>> >>>>>> Thanks >>>>>> Yumin >>>>>> >>>>>> >>>>> >>>> >>
