Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8264644: Add PrintClassLoaderDataGraphAtExit to print the detailed CLD graph #3323

Closed
wants to merge 6 commits into from

Conversation

@kelthuzadx
Copy link
Member

@kelthuzadx kelthuzadx commented Apr 2, 2021

Trivial chanage to make debugging happy, now cld->print() would be:

ClassLoaderData(0x00007ff17432b670)
 - name                'platform'
 - holder              WeakHandle: 0x00000007fef56678
 - class loader        0x7ff17432b828
 - metaspace           0x7ff17468a0b0
 - unloading           false
 - class mirror holder false
 - modified oops       true
 - keep alive          0
 - claim               strong
 - handles             43
 - dependency count    0
 - klasses             {java.sql.SQLFeatureNotSupportedException,java.sql.SQLNonTransientException,java.sql.SQLException,sun.util.resources.provider.NonBaseLocaleDataMetaInfo,org.ietf.jgss.GSSException,javax.sql.DataSource,java.sql.Wrapper,javax.sql.CommonDataSource,java.sql.Time,java.sql.Date,java.sql.Timestamp,sun.util.resources.cldr.provider.CLDRLocaleDataMetaInfo, }
 - packages            0x7ff17432bc20
 - module              0x7ff17432c520
 - unnamed module      0x7ff17432c3d0
 - dictionary          0x7ff17432c470
 - deallocate list     0x7ff0a4023bd0

Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issue

  • JDK-8264644: Add PrintClassLoaderDataGraphAtExit to print the detailed CLD graph

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/3323/head:pull/3323
$ git checkout pull/3323

Update a local copy of the PR:
$ git checkout pull/3323
$ git pull https://git.openjdk.java.net/jdk pull/3323/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 3323

View PR using the GUI difftool:
$ git pr show -t 3323

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/3323.diff

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Apr 2, 2021

👋 Welcome back yyang! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk openjdk bot added the rfr label Apr 2, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Apr 2, 2021

@kelthuzadx The following label will be automatically applied to this pull request:

  • hotspot

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

@openjdk openjdk bot added the hotspot label Apr 2, 2021
@mlbridge
Copy link

@mlbridge mlbridge bot commented Apr 2, 2021

@coleenp
coleenp approved these changes Apr 2, 2021
Copy link

@coleenp coleenp left a comment

This looks nice. If you use -XX:+PrintSystemDictionaryAtExit with this change, is it pages and pages long? Or does it look ok? Thank you for doing this!

if (!_holder.is_null()) {
out->print (" - holder ");
_holder.print_on(out);
out->print_cr("");

This comment has been minimized.

@coleenp

coleenp Apr 2, 2021

indent 2 here.

@openjdk
Copy link

@openjdk openjdk bot commented Apr 2, 2021

@kelthuzadx This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8264644: Add PrintClassLoaderDataGraphAtExit to print the detailed CLD graph

Reviewed-by: coleenp, dholmes, shade

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 119 new commits pushed to the master branch:

  • b1ebf82: 8264358: Don't create invalid oop in method handle tracing
  • 627ad9f: 8262328: Templatize JVMFlag boilerplate access methods
  • c15680e: 8264868: Reduce inclusion of registerMap.hpp and register.hpp
  • 5784f6b: 8264948: Check for TLS extensions total length
  • 42f4d70: 8264649: runtime/InternalApi/ThreadCpuTimesDeadlock.java crash in fastdebug C2 with -XX:-UseTLAB
  • 76bd313: 8264872: Dependencies: Migrate to PerfData counters
  • 07c8ff4: 8264871: Dependencies: Miscellaneous cleanups in dependencies.cpp
  • 863feab: 8005295: Use mandated information for printing of repeating annotations
  • f26cd2a: 8264997: Remove SystemDictionary::cache_get
  • 9ebc497: 8264765: BreakIterator sees bogus sentence boundary in parenthesized “i.e.” phrase
  • ... and 109 more: https://git.openjdk.java.net/jdk/compare/d2df9a7df89f095a9f706d849177eb201ac8d1cf...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@coleenp, @dholmes-ora, @shipilev) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

@openjdk openjdk bot added the ready label Apr 2, 2021
@kelthuzadx
Copy link
Member Author

@kelthuzadx kelthuzadx commented Apr 2, 2021

Hi @coleenp, thanks for looking at this, it does outputs too detailed content for klasses(actually that's what I want to see), if you think it is too much detail for most people, I can print detailed content of klasses of CLD only when -XX:+Verbose.

...
ClassLoaderData(0x00007fea483e5a50)
 - name                'app'
 - holder              WeakHandle: 0x00000007fef56d98
 - class loader        0x7fea483e5c08
 - metaspace           (nil)
 - unloading           false
 - class mirror holder false
 - modified oops       true
 - keep alive          0
 - claim               none
 - handles             20
 - dependency count    0
 - klasses             { }
 - packages            0x7fea483e5df0
 - module              0x7fea483ea640
 - unnamed module      0x7fea483e65a0
 - dictionary          0x7fea483e6640
 - deallocate list     (nil)
ClassLoaderData(0x00007fea482ee640)
 - name                'platform'
 - holder              WeakHandle: 0x00000007fef56678
 - class loader        0x7fea482ee7f8
 - metaspace           (nil)
 - unloading           false
 - class mirror holder false
 - modified oops       true
 - keep alive          0
 - claim               none
 - handles             24
 - dependency count    0
 - klasses             { }
 - packages            0x7fea482eebf0
 - module              0x7fea482ef4f0
 - unnamed module      0x7fea482ef3a0
 - dictionary          0x7fea482ef440
 - deallocate list     (nil)
ClassLoaderData(0x00007fea482e01f0)
 - name                'bootstrap'
 - class loader        (nil)
 - metaspace           0x7fea48391b60
 - unloading           false
 - class mirror holder false
 - modified oops       true
 - keep alive          1
 - claim               none
 - handles             668
 - dependency count    0
 - klasses             {java.lang.Shutdown$Lock,java.lang.Shutdown,[Ljava.nio.charset.CoderResult;,java.nio.charset.CoderResult,java.nio.HeapCharBuffer,java.nio.CharBuffer,java.lang.Readable,java.lang.invoke.StringConcatFactory$3,java.lang.invoke.StringConcatFactory$2,java.lang.invoke.StringConcatFactory$1,java.lang.invoke.StringConcatFactory,jdk.internal.util.Preconditions,sun.net.util.IPAddressUtil,sun.net.www.protocol.file.Handler,java.net.URLStreamHandler,java.util.HexFormat,sun.net.www.ParseUtil,[Ljava.io.File$PathStatus;,java.io.File$PathStatus,java.net.URL$3,jdk.internal.access.JavaNetURLAccess,java.net.URL$DefaultFactory,java.net.URLStreamHandlerFactory,jdk.internal.loader.URLClassPath,jdk.internal.loader.BootLoader,java.util.ArrayDeque,java.util.Deque,java.util.Queue,jdk.internal.loader.NativeLibraries$2,jdk.internal.loader.NativeLibraries$NativeLibraryImpl,jdk.internal.loader.NativeLibrary,java.security.ProtectionDomain$JavaSecurityAccessImpl,jdk.internal.access.JavaSecurityAccess,java.util.WeakHashMap$KeySet,java.util.Collections$SetFromMap,[Ljava.util.WeakHashMap$Entry;,java.util.WeakHashMap$Entry,java.util.WeakHashMap,java.lang.ClassLoader$ParallelLoaders,[Ljava.security.cert.Certificate;,java.security.cert.Certificate,java.net.URI$1,jdk.internal.access.JavaNetUriAccess,jdk.internal.loader.ClassLoaderValue,jdk.internal.loader.AbstractClassLoaderValue,jdk.internal.module.ModuleBootstrap$Counters,jdk.internal.module.ModulePatcher,jdk.internal.util.ArraysSupport,java.io.UnixFileSystem,java.io.FileSystem,java.io.DefaultFileSystem,java.io.File,java.lang.module.ModuleDescriptor$1,jdk.internal.access.JavaLangModuleAccess,java.lang.reflect.Modifier,sun.invoke.util.VerifyAccess,jdk.internal.module.ModuleBootstrap,java.lang.invoke.MethodHandleStatics,java.lang.IllegalArgumentException,java.util.Collections$EmptyMap,java.util.Collections$EmptyList,java.util.Collections$EmptySet,java.util.Collections,jdk.internal.misc.OSEnvironment,jdk.internal.misc.Signal$NativeHandler,[Ljava.util.Hashtable$Entry;,java.util.Hashtable$Entry,jdk.internal.misc.Signal,java.lang.Terminator$1,jdk.internal.misc.Signal$Handler,java.lang.Terminator,java.io.BufferedWriter,java.nio.ByteOrder,java.nio.HeapByteBuffer,java.nio.Buffer$1,jdk.internal.access.JavaNioAccess,jdk.internal.misc.ScopedMemoryAccess,java.nio.ByteBuffer,java.nio.charset.CodingErrorAction,sun.nio.cs.UTF_8$Encoder,java.nio.charset.CharsetEncoder,sun.nio.cs.StreamEncoder,sun.nio.cs.UTF_8,sun.nio.cs.Unicode,sun.nio.cs.HistoricallyNamedCharset,sun.security.action.GetPropertyAction,java.util.concurrent.atomic.AtomicInteger,java.lang.ThreadLocal,sun.nio.cs.StandardCharsets,java.nio.charset.spi.CharsetProvider,java.nio.charset.Charset,java.io.OutputStreamWriter,java.io.Writer,java.io.BufferedOutputStream,java.io.PrintStream,java.io.FilterOutputStream,java.io.BufferedInputStream,java.io.FilterInputStream,java.io.FileOutputStream,java.io.OutputStream,java.io.Flushable,java.io.FileDescriptor$1,jdk.internal.access.JavaIOFileDescriptorAccess,java.io.FileDescriptor,java.io.FileInputStream,jdk.internal.util.StaticProperty,java.util.HashMap$EntryIterator,java.util.HashMap$HashIterator,java.util.HashMap$EntrySet,java.lang.CharacterDataLatin1,java.lang.CharacterData,java.util.Arrays,java.lang.VersionProps,java.lang.StringConcatHelper,jdk.internal.util.SystemProps$Raw,jdk.internal.util.SystemProps,jdk.internal.misc.VM,java.lang.System$2,jdk.internal.access.JavaLangAccess,java.lang.ref.Finalizer$FinalizerThread,java.lang.ref.Reference$1,jdk.internal.access.JavaLangRefAccess,java.lang.ref.ReferenceQueue$Lock,java.lang.ref.ReferenceQueue$Null,java.lang.ref.ReferenceQueue,jdk.internal.ref.Cleaner,java.lang.ref.Reference$ReferenceHandler,jdk.internal.reflect.ReflectionFactory,jdk.internal.reflect.ReflectionFactory$GetReflectionFactoryAction,java.security.PrivilegedAction,java.util.concurrent.ConcurrentHashMap$ReservationNode,java.util.concurrent.locks.LockSupport,java.util.concurrent.ConcurrentHashMap$CounterCell,[Ljava.util.concurrent.ConcurrentHashMap$Segment;,java.util.concurrent.ConcurrentHashMap$Segment,[Ljava.util.concurrent.locks.ReentrantLock;,java.util.concurrent.locks.ReentrantLock,[Ljava.util.concurrent.locks.Lock;,java.util.concurrent.locks.Lock,java.lang.Runtime,java.util.HashMap$TreeNode,java.util.LinkedHashMap$Entry,java.util.KeyValueHolder,java.util.ImmutableCollections$MapN$MapNIterator,java.util.ImmutableCollections$MapN$1,java.lang.Math,jdk.internal.reflect.Reflection,java.util.Objects,java.lang.invoke.MethodHandles$Lookup,java.lang.StringLatin1,java.lang.reflect.ReflectPermission,java.security.BasicPermission,java.security.Permission,java.security.Guard,java.lang.invoke.MemberName$Factory,java.lang.invoke.MethodHandles,jdk.internal.access.SharedSecrets,java.lang.reflect.ReflectAccess,jdk.internal.access.JavaLangReflectAccess,jdk.internal.misc.CDS,java.lang.String$CaseInsensitiveComparator,java.util.Comparator,[Ljava.io.ObjectStreamField;,java.io.ObjectStreamField,[Ljava.util.jar.Manifest;,[Ljava.net.URL;,java.lang.ArithmeticException,java.lang.NullPointerException,[J,[[I,[I,[S,[B,[D,[F,[C,[Z,java.lang.Module$ArchivedData,jdk.internal.module.ArchivedBootLayer,jdk.internal.loader.BuiltinClassLoader$LoadedModule,java.util.HashSet,java.util.AbstractSet,[Ljdk.internal.module.ServicesCatalog$ServiceProvider;,jdk.internal.module.ServicesCatalog$ServiceProvider,java.util.concurrent.CopyOnWriteArrayList,[Ljdk.internal.module.ServicesCatalog;,jdk.internal.module.ServicesCatalog,[Ljava.util.concurrent.ConcurrentHashMap$Node;,java.util.concurrent.ConcurrentHashMap$Node,jdk.internal.loader.NativeLibraries,java.security.ProtectionDomain$Key,[Ljava.security.Principal;,java.security.Principal,jdk.internal.loader.ClassLoaders$BootClassLoader,jdk.internal.loader.ArchivedClassLoaders,[Ljdk.internal.math.FDBigInteger;,jdk.internal.math.FDBigInteger,java.lang.ModuleLayer,java.util.ImmutableCollections,jdk.internal.module.ModuleLoaderMap$Mapper,java.util.function.Function,[Ljava.lang.module.ResolvedModule;,java.lang.module.ResolvedModule,java.lang.module.Configuration,[Ljava.util.HashMap$Node;,java.util.HashMap$Node,[Ljava.util.Map$Entry;,java.util.Map$Entry,java.util.HashMap,java.util.Collections$UnmodifiableMap,[Ljdk.internal.module.ModuleHashes;,jdk.internal.module.ModuleHashes,[Ljdk.internal.module.ModuleTarget;,jdk.internal.module.ModuleTarget,java.util.ImmutableCollections$ListN,[Ljava.lang.module.ModuleDescriptor$Opens;,java.lang.module.ModuleDescriptor$Opens,jdk.internal.module.SystemModuleFinders$3,jdk.internal.module.ModuleHashes$HashSupplier,jdk.internal.module.SystemModuleFinders$2,java.util.function.Supplier,java.net.URI,java.util.ImmutableCollections$List12,java.util.ImmutableCollections$AbstractImmutableList,[Ljava.lang.module.ModuleDescriptor$Provides;,java.lang.module.ModuleDescriptor$Provides,[Ljava.lang.module.ModuleDescriptor$Exports;,java.lang.module.ModuleDescriptor$Exports,[Ljava.lang.module.ModuleDescriptor$Requires$Modifier;,java.lang.module.ModuleDescriptor$Requires$Modifier,[Ljava.lang.Enum;,java.lang.Enum,[Ljava.lang.module.ModuleDescriptor$Requires;,java.lang.module.ModuleDescriptor$Requires,java.util.ImmutableCollections$Set12,java.lang.module.ModuleDescriptor$Version,[Ljava.lang.module.ModuleDescriptor;,java.lang.module.ModuleDescriptor,jdk.internal.module.ModuleReferenceImpl,[Ljava.lang.module.ModuleReference;,java.lang.module.ModuleReference,java.util.ImmutableCollections$SetN,java.util.ImmutableCollections$AbstractImmutableSet,java.util.Set,java.util.ImmutableCollections$AbstractImmutableCollection,jdk.internal.module.SystemModuleFinders$SystemModuleFinder,java.lang.module.ModuleFinder,jdk.internal.module.ArchivedModuleGraph,[Lsun.util.locale.BaseLocale;,sun.util.locale.BaseLocale,java.util.ImmutableCollections$MapN,java.util.ImmutableCollections$AbstractImmutableMap,java.util.jar.Attributes$Name,java.lang.Character$CharacterCache,java.lang.Short$ShortCache,java.lang.Byte$ByteCache,java.lang.Long$LongCache,java.lang.Integer$IntegerCache,jdk.internal.vm.vector.VectorSupport$VectorShuffle,jdk.internal.vm.vector.VectorSupport$VectorMask,jdk.internal.vm.vector.VectorSupport$Vector,jdk.internal.vm.vector.VectorSupport$VectorPayload,jdk.internal.vm.vector.VectorSupport,java.lang.reflect.RecordComponent,java.util.Iterator,[Ljava.lang.Long;,java.lang.Long,[Ljava.lang.Integer;,java.lang.Integer,[Ljava.lang.Short;,java.lang.Short,[Ljava.lang.Byte;,java.lang.Byte,java.lang.Double,java.lang.Float,[Ljava.lang.Number;,java.lang.Number,[Ljava.lang.Character;,java.lang.Character,java.lang.Boolean,java.util.concurrent.locks.AbstractOwnableSynchronizer,java.lang.LiveStackFrameInfo,java.lang.LiveStackFrame,java.lang.StackFrameInfo,java.lang.StackWalker$StackFrame,java.lang.StackStreamFactory$AbstractStackWalker,java.lang.StackWalker,java.nio.Buffer,[Ljava.lang.StackTraceElement;,java.lang.StackTraceElement,java.util.ArrayList,java.util.RandomAccess,java.util.AbstractList,java.util.List,java.util.AbstractCollection,java.util.Collection,java.lang.Iterable,java.util.concurrent.ConcurrentHashMap,java.util.concurrent.ConcurrentMap,java.util.AbstractMap,java.security.CodeSource,jdk.internal.loader.ClassLoaders$PlatformClassLoader,jdk.internal.loader.ClassLoaders$AppClassLoader,jdk.internal.loader.ClassLoaders,jdk.internal.loader.BuiltinClassLoader,java.util.jar.Manifest,java.net.URL,java.io.ByteArrayInputStream,java.io.InputStream,java.io.Closeable,java.lang.AutoCloseable,jdk.internal.module.Modules,jdk.internal.misc.Unsafe,jdk.internal.misc.UnsafeConstants,java.lang.StringBuilder,java.lang.StringBuffer,java.lang.AbstractStringBuilder,java.lang.Appendable,java.lang.AssertionStatusDirectives,java.lang.invoke.VolatileCallSite,java.lang.invoke.MutableCallSite,java.lang.invoke.ConstantCallSite,java.lang.invoke.MethodHandleNatives$CallSiteContext,jdk.internal.invoke.NativeEntryPoint,java.lang.invoke.CallSite,java.lang.BootstrapMethodError,[Ljava.lang.invoke.MethodType;,java.lang.invoke.MethodType,[Ljava.lang.invoke.TypeDescriptor$OfMethod;,java.lang.invoke.TypeDescriptor$OfMethod,[Ljava.lang.invoke.LambdaForm;,java.lang.invoke.LambdaForm,java.lang.invoke.MethodHandleNatives,java.lang.invoke.ResolvedMethodName,java.lang.invoke.MemberName,java.lang.invoke.VarHandle,java.lang.invoke.DirectMethodHandle,[Ljava.lang.invoke.MethodHandle;,java.lang.invoke.MethodHandle,jdk.internal.reflect.NativeConstructorAccessorImpl,jdk.internal.reflect.CallerSensitive,[Ljava.lang.annotation.Annotation;,java.lang.annotation.Annotation,jdk.internal.reflect.UnsafeStaticFieldAccessorImpl,jdk.internal.reflect.UnsafeFieldAccessorImpl,jdk.internal.reflect.FieldAccessorImpl,jdk.internal.reflect.FieldAccessor,jdk.internal.reflect.ConstantPool,jdk.internal.reflect.DelegatingClassLoader,jdk.internal.reflect.ConstructorAccessorImpl,jdk.internal.reflect.ConstructorAccessor,jdk.internal.reflect.MethodAccessorImpl,jdk.internal.reflect.MethodAccessor,jdk.internal.reflect.MagicAccessorImpl,[Ljava.lang.reflect.Constructor;,java.lang.reflect.Constructor,[Ljava.lang.reflect.Method;,java.lang.reflect.Method,[Ljava.lang.reflect.Executable;,java.lang.reflect.Executable,java.lang.reflect.Parameter,java.lang.reflect.Field,[Ljava.lang.reflect.Member;,java.lang.reflect.Member,[Ljava.lang.reflect.AccessibleObject;,java.lang.reflect.AccessibleObject,[Ljava.lang.Module;,java.lang.Module,java.util.Properties,java.util.Hashtable,java.util.Map,java.util.Dictionary,[Ljava.lang.ThreadGroup;,java.lang.ThreadGroup,[Ljava.lang.Thread$UncaughtExceptionHandler;,java.lang.Thread$UncaughtExceptionHandler,[Ljava.lang.Thread;,java.lang.Thread,[Ljava.lang.Runnable;,java.lang.Runnable,java.lang.ref.Finalizer,java.lang.ref.PhantomReference,java.lang.ref.FinalReference,[Ljava.lang.ref.WeakReference;,java.lang.ref.WeakReference,[Ljava.lang.ref.SoftReference;,java.lang.ref.SoftReference,[Ljava.lang.ref.Reference;,java.lang.ref.Reference,java.lang.IllegalMonitorStateException,java.lang.StackOverflowError,[Ljava.lang.OutOfMemoryError;,java.lang.OutOfMemoryError,java.lang.InternalError,[Ljava.lang.VirtualMachineError;,java.lang.VirtualMachineError,java.lang.ArrayStoreException,java.lang.ClassCastException,java.lang.NoClassDefFoundError,java.lang.LinkageError,java.lang.Record,java.lang.ClassNotFoundException,java.lang.ReflectiveOperationException,java.security.SecureClassLoader,java.security.AccessController,java.security.AccessControlContext,[Ljava.security.ProtectionDomain;,java.security.ProtectionDomain,java.lang.SecurityManager,java.lang.RuntimeException,java.lang.Exception,java.lang.ThreadDeath,[Ljava.lang.Error;,java.lang.Error,[Ljava.lang.Throwable;,java.lang.Throwable,java.lang.System,[Ljava.lang.ClassLoader;,java.lang.ClassLoader,[Ljava.lang.Cloneable;,java.lang.Cloneable,[Ljava.lang.Class;,java.lang.Class,[Ljava.lang.invoke.TypeDescriptor$OfField;,java.lang.invoke.TypeDescriptor$OfField,[Ljava.lang.invoke.TypeDescriptor;,java.lang.invoke.TypeDescriptor,[Ljava.lang.reflect.Type;,java.lang.reflect.Type,[Ljava.lang.reflect.GenericDeclaration;,java.lang.reflect.GenericDeclaration,[Ljava.lang.reflect.AnnotatedElement;,java.lang.reflect.AnnotatedElement,[Ljava.lang.String;,java.lang.String,[Ljava.lang.constant.ConstantDesc;,java.lang.constant.ConstantDesc,[Ljava.lang.constant.Constable;,java.lang.constant.Constable,[Ljava.lang.CharSequence;,java.lang.CharSequence,[Ljava.lang.Comparable;,java.lang.Comparable,[Ljava.io.Serializable;,java.io.Serializable,[[Ljava.lang.Object;,[Ljava.lang.Object;,java.lang.Object, }
 - packages            0x7fea482e03a0
 - module              0x7fea482e5030
 - unnamed module      0x7fea482e0b50
 - dictionary          0x7fea482e0bf0
 - jmethod count       3
 - deallocate list     (nil)
@coleenp
Copy link

@coleenp coleenp commented Apr 2, 2021

I was just using PrintSystemDictionaryAtExit and wanted to ignore the CLDG information, which is already quite verbose for this option, so there was a lot of scrolling.

It's sort of a small change to add a nonproduct PrintClassLoaderDataGraphAtExit and use that to print the CLDG at exit and remove it from PrintSystemDictionaryAtExit. Then we can each get the info we want.

@kelthuzadx
Copy link
Member Author

@kelthuzadx kelthuzadx commented Apr 2, 2021

While adding the new flag PrintSystemDictionaryAtExit, I found that we may have missed acquiring CLDGraph_lock in some places.

extern "C" int print_loader_data_graph() {
ResourceMark rm;
ClassLoaderDataGraph::print_on(tty);

if (should_verify_subset(Verify_ClassLoaderDataGraph)) {
log_debug(gc, verify)("ClassLoaderDataGraph");
ClassLoaderDataGraph::verify();

It seems they are neither at a safepoint nor owned the CLDGraph_lock?

Thanks!
Yang

@kelthuzadx kelthuzadx changed the title 8264644: More complete ClassLoaderData for cld->print() 8264644: Add PrintClassLoaderDataGraphAtExit to print detailed CLD contents Apr 2, 2021
@kelthuzadx kelthuzadx changed the title 8264644: Add PrintClassLoaderDataGraphAtExit to print detailed CLD contents 8264644: Add PrintClassLoaderDataGraphAtExit to print detailed the CLD graph Apr 2, 2021
@openjdk openjdk bot removed the ready label Apr 2, 2021
@kelthuzadx kelthuzadx changed the title 8264644: Add PrintClassLoaderDataGraphAtExit to print detailed the CLD graph 8264644: Add PrintClassLoaderDataGraphAtExit to print the detailed CLD graph Apr 2, 2021
@openjdk openjdk bot added the ready label Apr 2, 2021
@coleenp
Copy link

@coleenp coleenp commented Apr 2, 2021

Yes, I like the new option. You're correct. There should be a CLDG_lock at both Universe::verify and debug.cpp. The latter is only called from the debugger so I didn't know if adding the lock would break anything, the former should have the lock. If you don't mind adding these and retesting, while waiting for your second reviewer (who might be having a holiday weekend), that would be great. Thanks!

Copy link
Member

@dholmes-ora dholmes-ora left a comment

Hi Yi,

Approval in principle, but some changes requested, please see below.

Thanks,
David

void Method::print_jmethod_ids(const ClassLoaderData* loader_data, outputStream* out) {
out->print(" jni_method_id count = %d", loader_data->jmethod_ids()->count_methods());
out->print("%d", loader_data->jmethod_ids()->count_methods());
}
Comment on lines 2258 to 2260

This comment has been minimized.

@dholmes-ora

dholmes-ora Apr 6, 2021
Member

This method is pointless if it only prints the count to the stream without any descriptive text. Just delete it and print the method count directly in the caller.

This comment has been minimized.

@kelthuzadx

kelthuzadx Apr 6, 2021
Author Member

The type of jmethod_ids is JNIMethodBlock, which is declared in method.cpp, we have to access its internal fields via print_jmethod_ids that declared in method.hpp. But if we only want to show jmethod_ids's address, then we can remove Method::print_jmethod_ids and print it directly. I'm not sure if others think jmethod_ids._count is important for their debugging, so I keep unchanged :)

This comment has been minimized.

@dholmes-ora

dholmes-ora Apr 6, 2021
Member

Sorry I don't see your point. I am suggesting that this:

out->print (" - jmethod count ");
Method::print_jmethod_ids(this, out);
out->print_cr("");

be replaced by:

out->print_cr(" - jmethod count %d", this->jmethod_ids()->count_methods());

This comment has been minimized.

@kelthuzadx

kelthuzadx Apr 6, 2021
Author Member

Hi David,

I means, count_methods() is inaccessible from classLoaderDataGraph.cpp because this->jmethod_ids() is type of JNIMethodBlock, it was defined in method.cpp, i.e.:

// method.cpp
class JNIMethodBlock {
...
public:
int count_methods(){...}
}

In order to access count_methods, we have to add an API in method.hpp and method.cpp:

// method.cpp
class JNIMethodBlock {
...
int count_methods(){...}
}

void Method::print_jmethod_ids(JNIMethodBlock* block,...){
  block->count_methods();
  ....
}
// method.hpp
void Method::print_jmethod_ids(JNIMethodBlock* block,...);

Thanks~
Yang

This comment has been minimized.

@dholmes-ora

dholmes-ora Apr 6, 2021
Member

Ah I see. In that case I'd suggest adding an API to get the count. If external code knows enough about jmethod_ids that it knows to print the count, then the count is worth exposing IMO. Otherwise restore the actual "jmethod count" text to the print function so it's a stand-alone print function again.
Aside: print_jmethod_ids() is badly named given it doesn't print any id's.

_holder.print_on(out);
out->print_cr("");
}
out->print_cr(" - class loader %p", _class_loader.ptr_raw());

This comment has been minimized.

@dholmes-ora

dholmes-ora Apr 6, 2021
Member

We don't use the %p in VM shared code as it behaves differently across platforms - use PTR_FORMAT.

kelthuzadx added 2 commits Apr 6, 2021
@kelthuzadx
Copy link
Member Author

@kelthuzadx kelthuzadx commented Apr 6, 2021

Thanks @coleenp @dholmes-ora for your reviews.

For @coleenp : It looks Universe::verify is already called at the safepoint. print_loader_data_graph really misses CLDG_lock, it crashes without acquiring this lock. I've added MutexLocker for it.

For @dholmes-ora: print_jmethod_ids now renamed to print_jmethod_ids_count. In order to prevent incorrect use, I did not add an API like Method::get_jmethod_ids_count. Method:: print_jmethod_ids should be enough at present.

@coleenp
coleenp approved these changes Apr 6, 2021
if (_jmethod_ids != NULL) {
Method::print_jmethod_ids(this, out);
out->print (" - jmethod count ");
Method::print_jmethod_ids_counts(this, out);

This comment has been minimized.

@coleenp

coleenp Apr 6, 2021

Yes, this is good. It was interesting at one point (to me) how many jmethodIds were in each CLD.

This comment has been minimized.

@dholmes-ora

dholmes-ora Apr 7, 2021
Member

Okay by me too. But it should be count not counts.

Copy link
Member

@dholmes-ora dholmes-ora left a comment

A couple of minor nits.

Thanks,
David

@@ -949,8 +949,8 @@ void ClassLoaderData::print_on(outputStream* out) const {
_holder.print_on(out);
out->print_cr("");
}
out->print_cr(" - class loader %p", _class_loader.ptr_raw());
out->print_cr(" - metaspace %p", _metaspace);
out->print_cr(" - class loader " PTR_FORMAT, p2i(_class_loader.ptr_raw()));

This comment has been minimized.

@dholmes-ora

dholmes-ora Apr 7, 2021
Member

I'm surprised the p2i's were added. If the values are pointers then p2i should not be needed. If using p2i then INTPTR_FORMAT is the correct format specifier to use.

This comment has been minimized.

@kelthuzadx

kelthuzadx Apr 7, 2021
Author Member

I'm not sure the original intentions of these two macros. It looks like the definitions of PRT_FORMAT and INTPTR_FORMAT are identical:

#ifdef _LP64
#define INTPTR_FORMAT "0x%016" PRIxPTR
#define PTR_FORMAT "0x%016" PRIxPTR
#else // !_LP64

Also I find many occurrences that using p2i while format specifier is PRT_FORMAT. If this is indeed wrong, I may fix them later.

This comment has been minimized.

@kelthuzadx

kelthuzadx Apr 7, 2021
Author Member

Even if a pointer is given, p2i is necessary for PRT_FORMAT and INTPTR_FORMAT specifier, the compiler will throw errors without p2i.

if (_jmethod_ids != NULL) {
Method::print_jmethod_ids(this, out);
out->print (" - jmethod count ");
Method::print_jmethod_ids_counts(this, out);

This comment has been minimized.

@dholmes-ora

dholmes-ora Apr 7, 2021
Member

Okay by me too. But it should be count not counts.

@mlbridge
Copy link

@mlbridge mlbridge bot commented Apr 7, 2021

Mailing list message from David Holmes on hotspot-dev:

On 7/04/2021 3:30 pm, Yi Yang wrote:

On Wed, 7 Apr 2021 05:07:54 GMT, David Holmes <dholmes at openjdk.org> wrote:

Yi Yang has updated the pull request incrementally with two additional commits since the last revision:

- add CLDG_lock
- use PTR_FORMAT

src/hotspot/share/classfile/classLoaderData.cpp line 952:

950: out->print_cr("");
951: }
952: out->print_cr(" - class loader " PTR_FORMAT, p2i(_class_loader.ptr_raw()));

I'm surprised the p2i's were added. If the values are pointers then p2i should not be needed. If using p2i then INTPTR_FORMAT is the correct format specifier to use.

I'm not sure the original intentions of these two macros. It looks like the definitions of PRT_FORMAT and INTPTR_FORMAT are identical:

https://github.com/openjdk/jdk/blob/c3abdc9aadc734053dbcc43d5294d5f16a0b0ce3/src/hotspot/share/utilities/globalDefinitions.hpp#L129-L132

Yes the ultimate definitions are currently identical. The issue is about
semantic correctness: if printing a pointer type use PTR_FORMAT; if
printing an integer representation of a pointer (as provided by p2i())
then use INTPTR_FORMAT.

Also I find many occurrences that using p2i while format specifier is PRT_FORMAT. If this is indeed wrong, I may fix them later.

Yes unfortunately there is a lot of inconsistency here. p2i was
introduced with:

http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/53a41e7cbe05

and it was a very large change. Since then, in parts of runtime at
least, we have tried to use them in a consistent way.

Thanks,
David

@kelthuzadx
Copy link
Member Author

@kelthuzadx kelthuzadx commented Apr 9, 2021

Hi @dholmes-ora , how about my latest update? If you think it's okay, I would like to integrate it.

Copy link
Member

@dholmes-ora dholmes-ora left a comment

Looks good. Sorry for the delay.

Thanks,
David

@kelthuzadx
Copy link
Member Author

@kelthuzadx kelthuzadx commented Apr 9, 2021

Thank you Coleen and David~

/integrate

@openjdk openjdk bot added the sponsor label Apr 9, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Apr 9, 2021

@kelthuzadx
Your change (at version 1eb6766) is now ready to be sponsored by a Committer.

Copy link
Contributor

@shipilev shipilev left a comment

Looks good with a minor nit.

};
void ClassLoaderData::print_on(outputStream* out) const {
Comment on lines 942 to 943

This comment has been minimized.

@shipilev

shipilev Apr 12, 2021
Contributor

These declarations should be separated by a newline, I think?

This comment has been minimized.

@kelthuzadx

kelthuzadx Apr 12, 2021
Author Member

Yes, I've added a new line. Thank you Aleksey.

@openjdk openjdk bot removed the sponsor label Apr 12, 2021
@kelthuzadx
Copy link
Member Author

@kelthuzadx kelthuzadx commented Apr 12, 2021

/integrate

@openjdk openjdk bot added the sponsor label Apr 12, 2021
@openjdk
Copy link

@openjdk openjdk bot commented Apr 12, 2021

@kelthuzadx
Your change (at version 070ca1a) is now ready to be sponsored by a Committer.

@shipilev
Copy link
Contributor

@shipilev shipilev commented Apr 12, 2021

/sponsor

@openjdk
Copy link

@openjdk openjdk bot commented Apr 12, 2021

@shipilev @kelthuzadx Since your change was applied there have been 119 commits pushed to the master branch:

  • b1ebf82: 8264358: Don't create invalid oop in method handle tracing
  • 627ad9f: 8262328: Templatize JVMFlag boilerplate access methods
  • c15680e: 8264868: Reduce inclusion of registerMap.hpp and register.hpp
  • 5784f6b: 8264948: Check for TLS extensions total length
  • 42f4d70: 8264649: runtime/InternalApi/ThreadCpuTimesDeadlock.java crash in fastdebug C2 with -XX:-UseTLAB
  • 76bd313: 8264872: Dependencies: Migrate to PerfData counters
  • 07c8ff4: 8264871: Dependencies: Miscellaneous cleanups in dependencies.cpp
  • 863feab: 8005295: Use mandated information for printing of repeating annotations
  • f26cd2a: 8264997: Remove SystemDictionary::cache_get
  • 9ebc497: 8264765: BreakIterator sees bogus sentence boundary in parenthesized “i.e.” phrase
  • ... and 109 more: https://git.openjdk.java.net/jdk/compare/d2df9a7df89f095a9f706d849177eb201ac8d1cf...master

Your commit was automatically rebased without conflicts.

Pushed as commit 440c34a.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

@kelthuzadx kelthuzadx deleted the kelthuzadx:make_debugging_happy branch Apr 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants