changeset 54550:1d953ae4d39f

8216136: Take Compile_lock for SystemDictionary::_modification_counter with safepoint check Summary: Transition compiler thread from native to VM state to check the Compile_lock with safepoint always. Reviewed-by: dlong, eosterlund, dholmes
author coleenp
date Thu, 24 Jan 2019 08:44:26 -0500
parents d927fc999582
children 9459533ef916
files src/hotspot/share/ci/ciEnv.cpp src/hotspot/share/ci/ciEnv.hpp src/hotspot/share/opto/callGenerator.cpp src/hotspot/share/opto/parse1.cpp src/hotspot/share/runtime/mutexLocker.cpp
diffstat 5 files changed, 14 insertions(+), 8 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/share/ci/ciEnv.cpp	Thu Jan 24 14:43:56 2019 +0100
+++ b/src/hotspot/share/ci/ciEnv.cpp	Thu Jan 24 08:44:26 2019 -0500
@@ -915,8 +915,15 @@
   return JavaThread::current()->thread_state() == _thread_in_vm;
 }
 
+bool ciEnv::system_dictionary_modification_counter_changed_locked() {
+  assert_locked_or_safepoint(Compile_lock);
+  return _system_dictionary_modification_counter != SystemDictionary::number_of_modifications();
+}
+
 bool ciEnv::system_dictionary_modification_counter_changed() {
-  return _system_dictionary_modification_counter != SystemDictionary::number_of_modifications();
+  VM_ENTRY_MARK;
+  MutexLocker ml(Compile_lock, THREAD); // lock with safepoint check
+  return system_dictionary_modification_counter_changed_locked();
 }
 
 // ------------------------------------------------------------------
@@ -927,7 +934,7 @@
 void ciEnv::validate_compile_task_dependencies(ciMethod* target) {
   if (failing())  return;  // no need for further checks
 
-  bool counter_changed = system_dictionary_modification_counter_changed();
+  bool counter_changed = system_dictionary_modification_counter_changed_locked();
   Dependencies::DepType result = dependencies()->validate_dependencies(_task, counter_changed);
   if (result != Dependencies::end_marker) {
     if (result == Dependencies::call_site_target_value) {
--- a/src/hotspot/share/ci/ciEnv.hpp	Thu Jan 24 14:43:56 2019 +0100
+++ b/src/hotspot/share/ci/ciEnv.hpp	Thu Jan 24 08:44:26 2019 -0500
@@ -292,6 +292,8 @@
   // respect to method dependencies (e.g. concurrent class loading).
   void validate_compile_task_dependencies(ciMethod* target);
 
+  // Call internally when Compile_lock is already held.
+  bool system_dictionary_modification_counter_changed_locked();
 public:
   enum {
     MethodCompilable,
--- a/src/hotspot/share/opto/callGenerator.cpp	Thu Jan 24 14:43:56 2019 +0100
+++ b/src/hotspot/share/opto/callGenerator.cpp	Thu Jan 24 08:44:26 2019 -0500
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2019, 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
@@ -90,7 +90,6 @@
   // Grab signature for matching/allocation
 #ifdef ASSERT
   if (parser.tf() != (parser.depth() == 1 ? C->tf() : tf())) {
-    MutexLockerEx ml(Compile_lock, Mutex::_no_safepoint_check_flag);
     assert(C->env()->system_dictionary_modification_counter_changed(),
            "Must invalidate if TypeFuncs differ");
   }
--- a/src/hotspot/share/opto/parse1.cpp	Thu Jan 24 14:43:56 2019 +0100
+++ b/src/hotspot/share/opto/parse1.cpp	Thu Jan 24 08:44:26 2019 -0500
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2019, 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
@@ -524,7 +524,6 @@
   if (depth() == 1) {
     assert(C->is_osr_compilation() == this->is_osr_parse(), "OSR in sync");
     if (C->tf() != tf()) {
-      MutexLockerEx ml(Compile_lock, Mutex::_no_safepoint_check_flag);
       assert(C->env()->system_dictionary_modification_counter_changed(),
              "Must invalidate if TypeFuncs differ");
     }
@@ -1041,7 +1040,6 @@
       // not compilable. Just using an assertion instead would be dangerous
       // as this could lead to an infinite compile loop in non-debug builds.
       {
-        MutexLockerEx ml(Compile_lock, Mutex::_no_safepoint_check_flag);
         if (C->env()->system_dictionary_modification_counter_changed()) {
           C->record_failure(C2Compiler::retry_class_loading_during_parsing());
         } else {
--- a/src/hotspot/share/runtime/mutexLocker.cpp	Thu Jan 24 14:43:56 2019 +0100
+++ b/src/hotspot/share/runtime/mutexLocker.cpp	Thu Jan 24 08:44:26 2019 -0500
@@ -306,7 +306,7 @@
   def(JvmtiThreadState_lock        , PaddedMutex  , nonleaf+2,   false, Monitor::_safepoint_check_always);     // Used by JvmtiThreadState/JvmtiEventController
   def(Management_lock              , PaddedMutex  , nonleaf+2,   false, Monitor::_safepoint_check_always);     // used for JVM management
 
-  def(Compile_lock                 , PaddedMutex  , nonleaf+3,   true,  Monitor::_safepoint_check_sometimes);
+  def(Compile_lock                 , PaddedMutex  , nonleaf+3,   true,  Monitor::_safepoint_check_always);
   def(MethodData_lock              , PaddedMutex  , nonleaf+3,   false, Monitor::_safepoint_check_always);
   def(TouchedMethodLog_lock        , PaddedMutex  , nonleaf+3,   false, Monitor::_safepoint_check_always);