Serguei, Webrev updated in-place (press shift-reload):
http://cr.openjdk.java.net/~dsamersoff/JDK-8151991/webrev.02/ -Dmitry On 2016-03-24 12:22, serguei.spit...@oracle.com wrote: > Hi Dmitry, > > -#if INCLUDE_SERVICES // Heap dumping/inspection supported > +#if INCLUDE_SERVICES > DCmdFactory::register_DCmdFactory(new > DCmdFactoryImpl<HeapDumpDCmd>(DCmd_Source_Internal | DCmd_Source_AttachAPI, > true, false)); > DCmdFactory::register_DCmdFactory(new > DCmdFactoryImpl<ClassHistogramDCmd>(full_export, true, false)); > DCmdFactory::register_DCmdFactory(new > DCmdFactoryImpl<ClassStatsDCmd>(full_export, true, false)); > DCmdFactory::register_DCmdFactory(new > DCmdFactoryImpl<ClassHierarchyDCmd>(full_export, true, false)); > DCmdFactory::register_DCmdFactory(new > DCmdFactoryImpl<SymboltableDCmd>(full_export, true, false)); > DCmdFactory::register_DCmdFactory(new > DCmdFactoryImpl<StringtableDCmd>(full_export, true, false)); > + DCmdFactory::register_DCmdFactory(new > DCmdFactoryImpl<JVMTIAgentLoadDCmd>(full_export, true, false)); > #endif // INCLUDE_SERVICES > #if INCLUDE_JVMTI > DCmdFactory::register_DCmdFactory(new > DCmdFactoryImpl<JVMTIDataDumpDCmd>(full_export, true, false)); > - DCmdFactory::register_DCmdFactory(new > DCmdFactoryImpl<JVMTIAgentLoadDCmd>(full_export, true, false)); > #endif // INCLUDE_JVMTI > > > The registration of the JVMTIAgentLoadDCmd has to be guarded with the > INCLUDE_JVMTI. > And now, it is not guarded. > > Otherwise, the fix looks good. > > > Thanks, > Serguei > > > > > On 3/23/16 10:43, Dmitry Samersoff wrote: >> Everybody, >> >> Webrev updated according to David's concerns. >> >> http://cr.openjdk.java.net/~dsamersoff/JDK-8151991/webrev.02/ >> >> -Dmitry >> >> On 2016-03-18 07:42, David Holmes wrote: >>> On 17/03/2016 12:14 AM, Dmitry Samersoff wrote: >>>> Everybody, >>>> >>>> Please, review small fix. >>>> >>>> http://cr.openjdk.java.net/~dsamersoff/JDK-8151991/webrev.01/ >>>> >>>> New diagnostic command (JVMTI.agent_load) should be guarded by >>>> #if INCLUDE_SERVICES and don't brake minimal VM build. >>> Initially I was confused as to why this was associated with >>> INCLUDE_SERVICES as it seems unrelated to the all the other things >>> guarded by INCLUDE_SERVICES. But now I see that the attachListener code >>> is completely excluded by INCLUDE_SERVICES (excludeSrc.gmk) so it makes >>> sense that the same guard is used in the diagnosticCommand sources (or >>> else an independent guard introduced?). >>> >>> However you would then also need the same guard in: >>> >>> src/share/vm/prims/jvmtiExport.cpp >>> >>> to allow INCLUDE_JVMTI to be true and INCLUDE_SERVICES to be false. >>> >>> lso can you update this comment: >>> >>> 64 #if INCLUDE_SERVICES // Heap dumping/inspection supported >>> >>> to also refer to the JVM TI agent/attach support >>> >>> Thanks, >>> David >>> ----- >>> >>> >>>> -Dmitry >>>> >> > -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
