-
Notifications
You must be signed in to change notification settings - Fork 6.1k
8329655: Cleanup KlassObj and klassOop names after the PermGen removal #18618
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -83,14 +83,14 @@ const char* KlassInfoEntry::name() const { | |
| if (_klass->name() != nullptr) { | ||
| name = _klass->external_name(); | ||
| } else { | ||
| if (_klass == Universe::boolArrayKlassObj()) name = "<boolArrayKlass>"; else | ||
| if (_klass == Universe::charArrayKlassObj()) name = "<charArrayKlass>"; else | ||
| if (_klass == Universe::floatArrayKlassObj()) name = "<floatArrayKlass>"; else | ||
| if (_klass == Universe::doubleArrayKlassObj()) name = "<doubleArrayKlass>"; else | ||
| if (_klass == Universe::byteArrayKlassObj()) name = "<byteArrayKlass>"; else | ||
| if (_klass == Universe::shortArrayKlassObj()) name = "<shortArrayKlass>"; else | ||
| if (_klass == Universe::intArrayKlassObj()) name = "<intArrayKlass>"; else | ||
| if (_klass == Universe::longArrayKlassObj()) name = "<longArrayKlass>"; else | ||
| if (_klass == Universe::boolArrayKlass()) name = "<boolArrayKlass>"; else | ||
| if (_klass == Universe::charArrayKlass()) name = "<charArrayKlass>"; else | ||
| if (_klass == Universe::floatArrayKlass()) name = "<floatArrayKlass>"; else | ||
| if (_klass == Universe::doubleArrayKlass()) name = "<doubleArrayKlass>"; else | ||
| if (_klass == Universe::byteArrayKlass()) name = "<byteArrayKlass>"; else | ||
| if (_klass == Universe::shortArrayKlass()) name = "<shortArrayKlass>"; else | ||
| if (_klass == Universe::intArrayKlass()) name = "<intArrayKlass>"; else | ||
| if (_klass == Universe::longArrayKlass()) name = "<longArrayKlass>"; else | ||
| name = "<no name>"; | ||
| } | ||
| return name; | ||
|
|
@@ -170,7 +170,7 @@ class KlassInfoTable::AllClassesFinder : public LockedClassesDo { | |
|
|
||
| KlassInfoTable::KlassInfoTable(bool add_all_classes) { | ||
| _size_of_instances_in_words = 0; | ||
| _ref = (HeapWord*) Universe::boolArrayKlassObj(); | ||
| _ref = (uintptr_t) Universe::boolArrayKlass(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems weird (non-obvious) to cast to uintptr_t here. I see it is only used in KlassInfoTable::hash(), which is weird, too. I am not sure that this even does a useful hashing. Might be worth to get rid of the whole thing and use the fastHash stuff that @rose00 proposed for Lilliput. Perhaps in a follow-up. I'd probably either cast to void* or Klass*, or cast to uintptr_t as you did and remove the unnecessary cast in ::hash(). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree. I'll start by removing the redundant cast in |
||
| _buckets = | ||
| (KlassInfoBucket*) AllocateHeap(sizeof(KlassInfoBucket) * _num_buckets, | ||
| mtInternal, CURRENT_PC, AllocFailStrategy::RETURN_NULL); | ||
|
|
@@ -196,7 +196,7 @@ KlassInfoTable::~KlassInfoTable() { | |
| } | ||
|
|
||
| uint KlassInfoTable::hash(const Klass* p) { | ||
| return (uint)(((uintptr_t)p - (uintptr_t)_ref) >> 2); | ||
| return (uint)(((uintptr_t)p - _ref) >> 2); | ||
| } | ||
|
|
||
| KlassInfoEntry* KlassInfoTable::lookup(Klass* k) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -108,9 +108,8 @@ class KlassInfoTable: public StackObj { | |
| size_t _size_of_instances_in_words; | ||
|
|
||
| // An aligned reference address (typically the least | ||
| // address in the perm gen) used for hashing klass | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rats I missed this. |
||
| // objects. | ||
| HeapWord* _ref; | ||
| // address in the metaspace) used for hashing klasses. | ||
| uintptr_t _ref; | ||
|
|
||
| KlassInfoBucket* _buckets; | ||
| uint hash(const Klass* p); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated, but what's the point of the %%% in all those comments? Might want to remove that, while you're there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is an old-style TODO. I'm considering if we shouldn't just remove these comments. What do people think about that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not even sure what they want to say, really. Should be good to remove, and if anybody can make sense of it, record an issue in the bug-tracker?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. I removed the %%%. I'll wait a little bit to see if someone else wants to keep them for some reason, if not, I'll remove them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think leaving these comments without the %%% seems fine. Describing this idea in a CR is a lot more difficult than seeing it in context as commentary, and unless the enhancement has other motivation, it won't be picked up. Leaving the comment as a clue seems useful.