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

8262046: Clean up parallel class loading code and comments #3200

Closed
wants to merge 6 commits into from
@@ -351,7 +351,7 @@ static inline void log_circularity_error(PlaceholderEntry* probe) {
// resolve_super_or_fail adds a LOAD_SUPER placeholder to the placeholder table before calling
// resolve_instance_class_or_null. ClassCircularityError is detected when a LOAD_SUPER or LOAD_INSTANCE
// placeholder for the same thread, class, classloader is found.
//
// This can be seen with logging option: -Xlog:class+load+placeholders=debug.
//
This conversation was marked as resolved by coleenp

This comment has been minimized.

@coleenp

coleenp Mar 30, 2021
Author

resolve_super_or_fail had this comment that I found difficult to understand and work through, which I did at one point, so I wanted to replace it with one that seemed less so. It occurs to me that this is probably unhelpful also. Maybe the whole comment from lines 351 to 381 should be replaced with a sentence like:

//resolve_super_or_fail adds a LOAD_SUPER placeholder to the placeholder table before calling
// resolve_instance_class_or_null. ClassCircularityError is detected when a LOAD_SUPER or LOAD_INSTANCE
// placeholder for the same thread, class, classloader is found.

I'm unlikely to work through the new comment example again myself.

This comment has been minimized.

@coleenp

coleenp Mar 31, 2021
Author

Since nobody wants to read this example in this comment and neither do I, I'm removing it. Debugging this using -Xlog:class+load+placeholders=debug will show the stacking of placeholders that detect CCE.

This comment has been minimized.

@iklam

iklam Mar 31, 2021
Member

I guess it's OK to remove this comment, since most of us who have looked at this code in the past few years haven't found this comment to be helpful. Maybe you can add a comment about -Xlog:class+load+placeholders=debug instead?

If we do want to improve the comment, maybe we can replacing Base->Super with something like Car->Vehicle. That way it's clear which is the supertype.

This comment has been minimized.

@coleenp

coleenp Apr 1, 2021
Author

Thanks Ioi.

InstanceKlass* SystemDictionary::resolve_super_or_fail(Symbol* class_name,
Symbol* super_name,
ProTip! Use n and p to navigate between commits in a pull request.