8202447: Fix unloading_occurred to mean unloading_occurred
Summary: nmethod unloading does not need to test for jvmti to set unloading_occurred, nor do we need to clean weak Klasses in metadata if unloading does not occur.
Reviewed-by: sspitsyn, rehn
--- a/src/hotspot/share/code/compiledMethod.cpp Thu May 03 08:07:20 2018 -0400
+++ b/src/hotspot/share/code/compiledMethod.cpp Fri Apr 27 15:00:04 2018 -0400
@@ -389,22 +389,24 @@
void CompiledMethod::clean_ic_if_metadata_is_dead(CompiledIC *ic) {
if (ic->is_icholder_call()) {
- // The only exception is compiledICHolder oops which may
+ // The only exception is compiledICHolder metdata which may
// yet be marked below. (We check this further below).
- CompiledICHolder* cichk_oop = ic->cached_icholder();
+ CompiledICHolder* cichk_metdata = ic->cached_icholder();
- if (cichk_oop->is_loader_alive()) {
+ if (cichk_metdata->is_loader_alive()) {
return;
}
} else {
- Metadata* ic_oop = ic->cached_metadata();
- if (ic_oop != NULL) {
- if (ic_oop->is_klass()) {
- if (((Klass*)ic_oop)->is_loader_alive()) {
+ Metadata* ic_metdata = ic->cached_metadata();
+ if (ic_metdata != NULL) {
+ if (ic_metdata->is_klass()) {
+ if (((Klass*)ic_metdata)->is_loader_alive()) {
return;
}
- } else if (ic_oop->is_method()) {
- if (((Method*)ic_oop)->method_holder()->is_loader_alive()) {
+ } else if (ic_metdata->is_method()) {
+ Method* method = (Method*)ic_metdata;
+ assert(!method->is_old(), "old method should have been cleaned");
+ if (method->method_holder()->is_loader_alive()) {
return;
}
} else {
@@ -493,16 +495,6 @@
// (See comment above.)
}
- // The RedefineClasses() API can cause the class unloading invariant
- // to no longer be true. See jvmtiExport.hpp for details.
- // Also, leave a debugging breadcrumb in local flag.
- if (JvmtiExport::has_redefined_a_class()) {
- // This set of the unloading_occurred flag is done before the
- // call to post_compiled_method_unload() so that the unloading
- // of this nmethod is reported.
- unloading_occurred = true;
- }
-
// Exception cache
clean_exception_cache();
@@ -581,16 +573,6 @@
// (See comment above.)
}
- // The RedefineClasses() API can cause the class unloading invariant
- // to no longer be true. See jvmtiExport.hpp for details.
- // Also, leave a debugging breadcrumb in local flag.
- if (JvmtiExport::has_redefined_a_class()) {
- // This set of the unloading_occurred flag is done before the
- // call to post_compiled_method_unload() so that the unloading
- // of this nmethod is reported.
- unloading_occurred = true;
- }
-
// Exception cache
clean_exception_cache();
--- a/src/hotspot/share/gc/cms/concurrentMarkSweepGeneration.cpp Thu May 03 08:07:20 2018 -0400
+++ b/src/hotspot/share/gc/cms/concurrentMarkSweepGeneration.cpp Fri Apr 27 15:00:04 2018 -0400
@@ -5245,7 +5245,7 @@
CodeCache::do_unloading(&_is_alive_closure, purged_class);
// Prune dead klasses from subklass/sibling/implementor lists.
- Klass::clean_weak_klass_links();
+ Klass::clean_weak_klass_links(purged_class);
}
{
--- a/src/hotspot/share/gc/g1/g1CollectedHeap.cpp Thu May 03 08:07:20 2018 -0400
+++ b/src/hotspot/share/gc/g1/g1CollectedHeap.cpp Fri Apr 27 15:00:04 2018 -0400
@@ -3529,6 +3529,7 @@
// To minimize the remark pause times, the tasks below are done in parallel.
class G1ParallelCleaningTask : public AbstractGangTask {
private:
+ bool _unloading_occurred;
G1StringAndSymbolCleaningTask _string_symbol_task;
G1CodeCacheUnloadingTask _code_cache_task;
G1KlassCleaningTask _klass_cleaning_task;
@@ -3541,6 +3542,7 @@
_string_symbol_task(is_alive, true, true, G1StringDedup::is_enabled()),
_code_cache_task(num_workers, is_alive, unloading_occurred),
_klass_cleaning_task(),
+ _unloading_occurred(unloading_occurred),
_resolved_method_cleaning_task() {
}
@@ -3566,7 +3568,11 @@
_code_cache_task.work_second_pass(worker_id);
// Clean all klasses that were not unloaded.
- _klass_cleaning_task.work();
+ // The weak metadata in klass doesn't need to be
+ // processed if there was no unloading.
+ if (_unloading_occurred) {
+ _klass_cleaning_task.work();
+ }
}
};
--- a/src/hotspot/share/gc/parallel/psMarkSweep.cpp Thu May 03 08:07:20 2018 -0400
+++ b/src/hotspot/share/gc/parallel/psMarkSweep.cpp Fri Apr 27 15:00:04 2018 -0400
@@ -565,7 +565,7 @@
CodeCache::do_unloading(is_alive_closure(), purged_class);
// Prune dead klasses from subklass/sibling/implementor lists.
- Klass::clean_weak_klass_links();
+ Klass::clean_weak_klass_links(purged_class);
}
{
--- a/src/hotspot/share/gc/parallel/psParallelCompact.cpp Thu May 03 08:07:20 2018 -0400
+++ b/src/hotspot/share/gc/parallel/psParallelCompact.cpp Fri Apr 27 15:00:04 2018 -0400
@@ -2140,7 +2140,7 @@
CodeCache::do_unloading(is_alive_closure(), purged_class);
// Prune dead klasses from subklass/sibling/implementor lists.
- Klass::clean_weak_klass_links();
+ Klass::clean_weak_klass_links(purged_class);
}
{
--- a/src/hotspot/share/gc/serial/genMarkSweep.cpp Thu May 03 08:07:20 2018 -0400
+++ b/src/hotspot/share/gc/serial/genMarkSweep.cpp Fri Apr 27 15:00:04 2018 -0400
@@ -234,7 +234,7 @@
CodeCache::do_unloading(&is_alive, purged_class);
// Prune dead klasses from subklass/sibling/implementor lists.
- Klass::clean_weak_klass_links();
+ Klass::clean_weak_klass_links(purged_class);
}
{
--- a/src/hotspot/share/oops/instanceKlass.hpp Thu May 03 08:07:20 2018 -0400
+++ b/src/hotspot/share/oops/instanceKlass.hpp Fri Apr 27 15:00:04 2018 -0400
@@ -1150,9 +1150,11 @@
#endif // INCLUDE_JVMTI
void clean_weak_instanceklass_links();
+ private:
void clean_implementors_list();
void clean_method_data();
+ public:
// Explicit metaspace deallocation of fields
// For RedefineClasses and class file parsing errors, we need to deallocate
// instanceKlasses and the metadata they point to.
--- a/src/hotspot/share/oops/klass.cpp Thu May 03 08:07:20 2018 -0400
+++ b/src/hotspot/share/oops/klass.cpp Fri Apr 27 15:00:04 2018 -0400
@@ -384,8 +384,8 @@
debug_only(verify();)
}
-void Klass::clean_weak_klass_links(bool clean_alive_klasses) {
- if (!ClassUnloading) {
+void Klass::clean_weak_klass_links(bool unloading_occurred, bool clean_alive_klasses) {
+ if (!ClassUnloading || !unloading_occurred) {
return;
}
--- a/src/hotspot/share/oops/klass.hpp Thu May 03 08:07:20 2018 -0400
+++ b/src/hotspot/share/oops/klass.hpp Fri Apr 27 15:00:04 2018 -0400
@@ -638,9 +638,9 @@
// Klass is considered alive. Has already been marked as unloading.
bool is_loader_alive() const { return !class_loader_data()->is_unloading(); }
- static void clean_weak_klass_links(bool clean_alive_klasses = true);
+ static void clean_weak_klass_links(bool unloading_occurred, bool clean_alive_klasses = true);
static void clean_subklass_tree() {
- clean_weak_klass_links(false /* clean_alive_klasses */);
+ clean_weak_klass_links(/*unloading_occurred*/ true , /* clean_alive_klasses */ false);
}
// GC specific object visitors
--- a/src/hotspot/share/oops/methodData.cpp Thu May 03 08:07:20 2018 -0400
+++ b/src/hotspot/share/oops/methodData.cpp Fri Apr 27 15:00:04 2018 -0400
@@ -429,7 +429,7 @@
ReceiverTypeData::clean_weak_method_links();
for (uint row = 0; row < method_row_limit(); row++) {
Method* p = method(row);
- if (p != NULL && !p->on_stack()) {
+ if (p != NULL && p->is_old()) {
clear_method_row(row);
}
}
@@ -1770,6 +1770,8 @@
verify_extra_data_clean(&cl);
}
+// This is called during redefinition to clean all "old" redefined
+// methods out of MethodData for all methods.
void MethodData::clean_weak_method_links() {
ResourceMark rm;
for (ProfileData* data = first_data();
--- a/src/hotspot/share/prims/jvmtiExport.hpp Thu May 03 08:07:20 2018 -0400
+++ b/src/hotspot/share/prims/jvmtiExport.hpp Fri Apr 27 15:00:04 2018 -0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1998, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2018, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -170,19 +170,6 @@
static void post_dynamic_code_generated(JvmtiEnv* env, const char *name, const void *code_begin,
const void *code_end) NOT_JVMTI_RETURN;
- // The RedefineClasses() API breaks some invariants in the "regular"
- // system. For example, there are sanity checks when GC'ing nmethods
- // that require the containing class to be unloading. However, when a
- // method is redefined, the old method and nmethod can become GC'able
- // without the containing class unloading. The state of becoming
- // GC'able can be asynchronous to the RedefineClasses() call since
- // the old method may still be running and cannot be GC'ed until
- // after all old invocations have finished. Additionally, a method
- // that has not been redefined may have an nmethod that depends on
- // the redefined method. The dependent nmethod will get deopted in
- // this case and may also be GC'able without the containing class
- // being unloaded.
- //
// This flag indicates whether RedefineClasses() has ever redefined
// one or more classes during the lifetime of the VM. The flag should
// only be set by the friend class and can be queried by other sub