Skip to content

Commit b961f25

Browse files
committed
8267191: Avoid repeated SystemDictionaryShared::should_be_excluded calls
Reviewed-by: dholmes, coleenp
1 parent 74f30ad commit b961f25

File tree

2 files changed

+55
-49
lines changed

2 files changed

+55
-49
lines changed

src/hotspot/share/classfile/systemDictionaryShared.cpp

Lines changed: 52 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ bool SystemDictionaryShared::_dump_in_progress = false;
8080
class DumpTimeSharedClassInfo: public CHeapObj<mtClass> {
8181
bool _excluded;
8282
bool _is_early_klass;
83+
bool _has_checked_exclusion;
8384
public:
8485
struct DTLoaderConstraint {
8586
Symbol* _name;
@@ -122,6 +123,7 @@ class DumpTimeSharedClassInfo: public CHeapObj<mtClass> {
122123
_nest_host = NULL;
123124
_failed_verification = false;
124125
_is_archived_lambda_proxy = false;
126+
_has_checked_exclusion = false;
125127
_id = -1;
126128
_clsfile_size = -1;
127129
_clsfile_crc32 = -1;
@@ -174,10 +176,6 @@ class DumpTimeSharedClassInfo: public CHeapObj<mtClass> {
174176
}
175177
}
176178

177-
void set_excluded() {
178-
_excluded = true;
179-
}
180-
181179
bool is_excluded() {
182180
// _klass may become NULL due to DynamicArchiveBuilder::set_to_null
183181
return _excluded || _failed_verification || _klass == NULL;
@@ -188,21 +186,14 @@ class DumpTimeSharedClassInfo: public CHeapObj<mtClass> {
188186
return _is_early_klass;
189187
}
190188

191-
void set_failed_verification() {
192-
_failed_verification = true;
193-
}
194-
195-
bool failed_verification() {
196-
return _failed_verification;
197-
}
198-
199-
void set_nest_host(InstanceKlass* nest_host) {
200-
_nest_host = nest_host;
201-
}
202-
203-
InstanceKlass* nest_host() {
204-
return _nest_host;
205-
}
189+
// simple accessors
190+
void set_excluded() { _excluded = true; }
191+
bool has_checked_exclusion() const { return _has_checked_exclusion; }
192+
void set_has_checked_exclusion() { _has_checked_exclusion = true; }
193+
bool failed_verification() const { return _failed_verification; }
194+
void set_failed_verification() { _failed_verification = true; }
195+
InstanceKlass* nest_host() const { return _nest_host; }
196+
void set_nest_host(InstanceKlass* nest_host) { _nest_host = nest_host; }
206197
};
207198

208199
inline unsigned DumpTimeSharedClassTable_hash(InstanceKlass* const& k) {
@@ -1326,41 +1317,60 @@ bool SystemDictionaryShared::is_early_klass(InstanceKlass* ik) {
13261317
return (info != NULL) ? info->is_early_klass() : false;
13271318
}
13281319

1329-
void SystemDictionaryShared::warn_excluded(InstanceKlass* k, const char* reason) {
1320+
// Returns true so the caller can do: return warn_excluded(".....");
1321+
bool SystemDictionaryShared::warn_excluded(InstanceKlass* k, const char* reason) {
13301322
ResourceMark rm;
13311323
log_warning(cds)("Skipping %s: %s", k->name()->as_C_string(), reason);
1324+
return true;
13321325
}
13331326

1334-
bool SystemDictionaryShared::should_be_excluded(InstanceKlass* k) {
1327+
bool SystemDictionaryShared::check_for_exclusion(InstanceKlass* k, DumpTimeSharedClassInfo* info) {
1328+
if (MetaspaceShared::is_in_shared_metaspace(k)) {
1329+
// We have reached a super type that's already in the base archive. Treat it
1330+
// as "not excluded".
1331+
assert(DynamicDumpSharedSpaces, "must be");
1332+
return false;
1333+
}
1334+
1335+
if (info == NULL) {
1336+
info = _dumptime_table->get(k);
1337+
assert(info != NULL, "supertypes of any classes in _dumptime_table must either be shared, or must also be in _dumptime_table");
1338+
}
1339+
1340+
if (!info->has_checked_exclusion()) {
1341+
if (check_for_exclusion_impl(k)) {
1342+
info->set_excluded();
1343+
}
1344+
info->set_has_checked_exclusion();
1345+
}
13351346

1347+
return info->is_excluded();
1348+
}
1349+
1350+
bool SystemDictionaryShared::check_for_exclusion_impl(InstanceKlass* k) {
13361351
if (k->is_in_error_state()) {
1337-
warn_excluded(k, "In error state");
1338-
return true;
1352+
return warn_excluded(k, "In error state");
13391353
}
13401354
if (k->has_been_redefined()) {
1341-
warn_excluded(k, "Has been redefined");
1342-
return true;
1355+
return warn_excluded(k, "Has been redefined");
13431356
}
13441357
if (!k->is_hidden() && k->shared_classpath_index() < 0 && is_builtin(k)) {
13451358
// These are classes loaded from unsupported locations (such as those loaded by JVMTI native
13461359
// agent during dump time).
1347-
warn_excluded(k, "Unsupported location");
1348-
return true;
1360+
return warn_excluded(k, "Unsupported location");
13491361
}
13501362
if (k->signers() != NULL) {
13511363
// We cannot include signed classes in the archive because the certificates
13521364
// used during dump time may be different than those used during
13531365
// runtime (due to expiration, etc).
1354-
warn_excluded(k, "Signed JAR");
1355-
return true;
1366+
return warn_excluded(k, "Signed JAR");
13561367
}
13571368
if (is_jfr_event_class(k)) {
13581369
// We cannot include JFR event classes because they need runtime-specific
13591370
// instrumentation in order to work with -XX:FlightRecorderOptions:retransform=false.
13601371
// There are only a small number of these classes, so it's not worthwhile to
13611372
// support them and make CDS more complicated.
1362-
warn_excluded(k, "JFR event class");
1363-
return true;
1373+
return warn_excluded(k, "JFR event class");
13641374
}
13651375
if (k->init_state() < InstanceKlass::linked) {
13661376
// In CDS dumping, we will attempt to link all classes. Those that fail to link will
@@ -1374,12 +1384,10 @@ bool SystemDictionaryShared::should_be_excluded(InstanceKlass* k) {
13741384
// a custom ClassLoader.loadClass() may be called, at a point where the
13751385
// class loader doesn't expect it.
13761386
if (has_class_failed_verification(k)) {
1377-
warn_excluded(k, "Failed verification");
1378-
return true;
1387+
return warn_excluded(k, "Failed verification");
13791388
} else {
13801389
if (!k->has_old_class_version()) {
1381-
warn_excluded(k, "Not linked");
1382-
return true;
1390+
return warn_excluded(k, "Not linked");
13831391
}
13841392
}
13851393
}
@@ -1393,34 +1401,33 @@ bool SystemDictionaryShared::should_be_excluded(InstanceKlass* k) {
13931401
}
13941402

13951403
if (k->has_old_class_version() && k->is_linked()) {
1396-
warn_excluded(k, "Old class has been linked");
1397-
return true;
1404+
return warn_excluded(k, "Old class has been linked");
13981405
}
13991406

1400-
InstanceKlass* super = k->java_super();
1401-
if (super != NULL && should_be_excluded(super)) {
1407+
if (k->is_hidden() && !is_registered_lambda_proxy_class(k)) {
14021408
ResourceMark rm;
1403-
log_warning(cds)("Skipping %s: super class %s is excluded", k->name()->as_C_string(), super->name()->as_C_string());
1409+
log_debug(cds)("Skipping %s: Hidden class", k->name()->as_C_string());
14041410
return true;
14051411
}
14061412

1407-
if (k->is_hidden() && !is_registered_lambda_proxy_class(k)) {
1413+
InstanceKlass* super = k->java_super();
1414+
if (super != NULL && check_for_exclusion(super, NULL)) {
14081415
ResourceMark rm;
1409-
log_debug(cds)("Skipping %s: %s", k->name()->as_C_string(), "Hidden class");
1416+
log_warning(cds)("Skipping %s: super class %s is excluded", k->name()->as_C_string(), super->name()->as_C_string());
14101417
return true;
14111418
}
14121419

14131420
Array<InstanceKlass*>* interfaces = k->local_interfaces();
14141421
int len = interfaces->length();
14151422
for (int i = 0; i < len; i++) {
14161423
InstanceKlass* intf = interfaces->at(i);
1417-
if (should_be_excluded(intf)) {
1424+
if (check_for_exclusion(intf, NULL)) {
14181425
log_warning(cds)("Skipping %s: interface %s is excluded", k->name()->as_C_string(), intf->name()->as_C_string());
14191426
return true;
14201427
}
14211428
}
14221429

1423-
return false;
1430+
return false; // false == k should NOT be excluded
14241431
}
14251432

14261433
// k is a class before relocating by ArchiveBuilder
@@ -1447,9 +1454,7 @@ void SystemDictionaryShared::validate_before_archiving(InstanceKlass* k) {
14471454
class ExcludeDumpTimeSharedClasses : StackObj {
14481455
public:
14491456
bool do_entry(InstanceKlass* k, DumpTimeSharedClassInfo& info) {
1450-
if (SystemDictionaryShared::should_be_excluded(k) || info.is_excluded()) {
1451-
info.set_excluded();
1452-
}
1457+
SystemDictionaryShared::check_for_exclusion(k, &info);
14531458
return true; // keep on iterating
14541459
}
14551460
};

src/hotspot/share/classfile/systemDictionaryShared.hpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,8 +212,8 @@ class SystemDictionaryShared: public SystemDictionary {
212212
static void write_lambda_proxy_class_dictionary(LambdaProxyClassDictionary* dictionary);
213213
static bool is_jfr_event_class(InstanceKlass *k);
214214
static bool is_registered_lambda_proxy_class(InstanceKlass* ik);
215-
static void warn_excluded(InstanceKlass* k, const char* reason);
216-
static bool should_be_excluded(InstanceKlass* k);
215+
static bool warn_excluded(InstanceKlass* k, const char* reason);
216+
static bool check_for_exclusion_impl(InstanceKlass* k);
217217

218218
static bool _dump_in_progress;
219219
DEBUG_ONLY(static bool _no_class_loading_should_happen;)
@@ -304,6 +304,7 @@ class SystemDictionaryShared: public SystemDictionary {
304304
return (k->shared_classpath_index() != UNREGISTERED_INDEX);
305305
}
306306
static void check_excluded_classes();
307+
static bool check_for_exclusion(InstanceKlass* k, DumpTimeSharedClassInfo* info);
307308
static void validate_before_archiving(InstanceKlass* k);
308309
static bool is_excluded_class(InstanceKlass* k);
309310
static void set_excluded(InstanceKlass* k);

0 commit comments

Comments
 (0)