Skip to content

Commit 1ebd946

Browse files
committed
8270333: -XX:+VerifyStringTableAtExit should not do linear search
Reviewed-by: dholmes, minqi
1 parent 04b73bc commit 1ebd946

File tree

3 files changed

+37
-21
lines changed

3 files changed

+37
-21
lines changed

src/hotspot/share/classfile/javaClasses.cpp

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -502,7 +502,7 @@ jchar* java_lang_String::as_unicode_string_or_null(oop java_string, int& length)
502502
return result;
503503
}
504504

505-
unsigned int java_lang_String::hash_code(oop java_string) {
505+
inline unsigned int java_lang_String::hash_code_impl(oop java_string, bool update) {
506506
// The hash and hashIsZero fields are subject to a benign data race,
507507
// making it crucial to ensure that any observable result of the
508508
// calculation in this method stays correct under any possible read of
@@ -529,14 +529,25 @@ unsigned int java_lang_String::hash_code(oop java_string) {
529529
}
530530
}
531531

532-
if (hash != 0) {
533-
java_string->int_field_put(_hash_offset, hash);
534-
} else {
535-
java_string->bool_field_put(_hashIsZero_offset, true);
532+
if (update) {
533+
if (hash != 0) {
534+
java_string->int_field_put(_hash_offset, hash);
535+
} else {
536+
java_string->bool_field_put(_hashIsZero_offset, true);
537+
}
536538
}
537539
return hash;
538540
}
539541

542+
unsigned int java_lang_String::hash_code(oop java_string) {
543+
return hash_code_impl(java_string, /*update=*/true);
544+
}
545+
546+
unsigned int java_lang_String::hash_code_noupdate(oop java_string) {
547+
return hash_code_impl(java_string, /*update=*/false);
548+
}
549+
550+
540551
char* java_lang_String::as_quoted_ascii(oop java_string) {
541552
typeArrayOop value = java_lang_String::value(java_string);
542553
int length = java_lang_String::length(java_string, value);

src/hotspot/share/classfile/javaClasses.hpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,8 @@ class java_lang_String : AllStatic {
127127
// returning true if the bit was already set.
128128
static bool test_and_set_flag(oop java_string, uint8_t flag_mask);
129129

130+
static inline unsigned int hash_code_impl(oop java_string, bool update);
131+
130132
public:
131133

132134
// Coders
@@ -222,6 +224,7 @@ class java_lang_String : AllStatic {
222224
}
223225

224226
static unsigned int hash_code(oop java_string);
227+
static unsigned int hash_code_noupdate(oop java_string);
225228

226229
static bool equals(oop java_string, const jchar* chars, int len);
227230
static bool equals(oop str1, oop str2);

src/hotspot/share/classfile/stringTable.cpp

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
#include "utilities/concurrentHashTable.inline.hpp"
5454
#include "utilities/concurrentHashTableTasks.inline.hpp"
5555
#include "utilities/macros.hpp"
56+
#include "utilities/resizeableResourceHash.hpp"
5657
#include "utilities/utf8.hpp"
5758

5859
// We prefer short chains of avg 2
@@ -597,39 +598,40 @@ void StringTable::verify() {
597598

598599
// Verification and comp
599600
class VerifyCompStrings : StackObj {
600-
GrowableArray<oop>* _oops;
601+
static unsigned string_hash(oop const& str) {
602+
return java_lang_String::hash_code_noupdate(str);
603+
}
604+
static bool string_equals(oop const& a, oop const& b) {
605+
return java_lang_String::equals(a, b);
606+
}
607+
608+
ResizeableResourceHashtable<oop, bool,
609+
ResourceObj::C_HEAP, mtInternal,
610+
string_hash, string_equals> _table;
601611
public:
602612
size_t _errors;
603-
VerifyCompStrings(GrowableArray<oop>* oops) : _oops(oops), _errors(0) {}
613+
VerifyCompStrings() : _table(unsigned(_items_count / 8) + 1), _errors(0) {}
604614
bool operator()(WeakHandle* val) {
605615
oop s = val->resolve();
606616
if (s == NULL) {
607617
return true;
608618
}
609-
int len = _oops->length();
610-
for (int i = 0; i < len; i++) {
611-
bool eq = java_lang_String::equals(s, _oops->at(i));
612-
assert(!eq, "Duplicate strings");
613-
if (eq) {
614-
_errors++;
615-
}
619+
bool created;
620+
_table.put_if_absent(s, true, &created);
621+
assert(created, "Duplicate strings");
622+
if (!created) {
623+
_errors++;
616624
}
617-
_oops->push(s);
618625
return true;
619626
};
620627
};
621628

622629
size_t StringTable::verify_and_compare_entries() {
623630
Thread* thr = Thread::current();
624-
GrowableArray<oop>* oops =
625-
new (ResourceObj::C_HEAP, mtInternal)
626-
GrowableArray<oop>((int)_current_size, mtInternal);
627-
628-
VerifyCompStrings vcs(oops);
631+
VerifyCompStrings vcs;
629632
if (!_local_table->try_scan(thr, vcs)) {
630633
log_info(stringtable)("verify unavailable at this moment");
631634
}
632-
delete oops;
633635
return vcs._errors;
634636
}
635637

0 commit comments

Comments
 (0)