8038212: Method::is_valid_method() check has performance regression impact for stackwalking

Thu, 15 May 2014 18:23:26 -0400

author
coleenp
date
Thu, 15 May 2014 18:23:26 -0400
changeset 6678
7384f6a12fc1
parent 6677
366c198c896d
child 6679
968a17f18337
child 6680
78bbf4d43a14

8038212: Method::is_valid_method() check has performance regression impact for stackwalking
Summary: Only prune metaspace virtual spaces at safepoint so walking them is safe outside a safepoint.
Reviewed-by: mgerdin, mgronlun, hseigel, stefank

src/share/vm/classfile/classLoaderData.cpp file | annotate | diff | comparison | revisions
src/share/vm/classfile/classLoaderData.hpp file | annotate | diff | comparison | revisions
src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp file | annotate | diff | comparison | revisions
src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp file | annotate | diff | comparison | revisions
src/share/vm/memory/allocation.cpp file | annotate | diff | comparison | revisions
src/share/vm/memory/metaspace.cpp file | annotate | diff | comparison | revisions
src/share/vm/memory/metaspace.hpp file | annotate | diff | comparison | revisions
src/share/vm/oops/klass.cpp file | annotate | diff | comparison | revisions
src/share/vm/oops/method.cpp file | annotate | diff | comparison | revisions
src/share/vm/oops/method.hpp file | annotate | diff | comparison | revisions
src/share/vm/runtime/os.cpp file | annotate | diff | comparison | revisions
src/share/vm/runtime/safepoint.cpp file | annotate | diff | comparison | revisions
src/share/vm/utilities/globalDefinitions.hpp file | annotate | diff | comparison | revisions
     1.1 --- a/src/share/vm/classfile/classLoaderData.cpp	Thu May 15 09:25:27 2014 -0400
     1.2 +++ b/src/share/vm/classfile/classLoaderData.cpp	Thu May 15 18:23:26 2014 -0400
     1.3 @@ -533,6 +533,8 @@
     1.4  ClassLoaderData* ClassLoaderDataGraph::_unloading = NULL;
     1.5  ClassLoaderData* ClassLoaderDataGraph::_saved_head = NULL;
     1.6  
     1.7 +bool ClassLoaderDataGraph::_should_purge = false;
     1.8 +
     1.9  // Add a new class loader data node to the list.  Assign the newly created
    1.10  // ClassLoaderData into the java/lang/ClassLoader object as a hidden field
    1.11  ClassLoaderData* ClassLoaderDataGraph::add(Handle loader, bool is_anonymous, TRAPS) {
    1.12 @@ -655,32 +657,6 @@
    1.13    return array;
    1.14  }
    1.15  
    1.16 -// For profiling and hsfind() only.  Otherwise, this is unsafe (and slow).  This
    1.17 -// is done lock free to avoid lock inversion problems.  It is safe because
    1.18 -// new ClassLoaderData are added to the end of the CLDG, and only removed at
    1.19 -// safepoint.  The _unloading list can be deallocated concurrently with CMS so
    1.20 -// this doesn't look in metaspace for classes that have been unloaded.
    1.21 -bool ClassLoaderDataGraph::contains(const void* x) {
    1.22 -  if (DumpSharedSpaces) {
    1.23 -    // There are only two metaspaces to worry about.
    1.24 -    ClassLoaderData* ncld = ClassLoaderData::the_null_class_loader_data();
    1.25 -    return (ncld->ro_metaspace()->contains(x) || ncld->rw_metaspace()->contains(x));
    1.26 -  }
    1.27 -
    1.28 -  if (UseSharedSpaces && MetaspaceShared::is_in_shared_space(x)) {
    1.29 -    return true;
    1.30 -  }
    1.31 -
    1.32 -  for (ClassLoaderData* cld = _head; cld != NULL; cld = cld->next()) {
    1.33 -    if (cld->metaspace_or_null() != NULL && cld->metaspace_or_null()->contains(x)) {
    1.34 -      return true;
    1.35 -    }
    1.36 -  }
    1.37 -
    1.38 -  // Do not check unloading list because deallocation can be concurrent.
    1.39 -  return false;
    1.40 -}
    1.41 -
    1.42  #ifndef PRODUCT
    1.43  bool ClassLoaderDataGraph::contains_loader_data(ClassLoaderData* loader_data) {
    1.44    for (ClassLoaderData* data = _head; data != NULL; data = data->next()) {
    1.45 @@ -739,6 +715,7 @@
    1.46  }
    1.47  
    1.48  void ClassLoaderDataGraph::purge() {
    1.49 +  assert(SafepointSynchronize::is_at_safepoint(), "must be at safepoint!");
    1.50    ClassLoaderData* list = _unloading;
    1.51    _unloading = NULL;
    1.52    ClassLoaderData* next = list;
     2.1 --- a/src/share/vm/classfile/classLoaderData.hpp	Thu May 15 09:25:27 2014 -0400
     2.2 +++ b/src/share/vm/classfile/classLoaderData.hpp	Thu May 15 18:23:26 2014 -0400
     2.3 @@ -66,6 +66,7 @@
     2.4    static ClassLoaderData* _unloading;
     2.5    // CMS support.
     2.6    static ClassLoaderData* _saved_head;
     2.7 +  static bool _should_purge;
     2.8  
     2.9    static ClassLoaderData* add(Handle class_loader, bool anonymous, TRAPS);
    2.10    static void post_class_unload_events(void);
    2.11 @@ -86,12 +87,20 @@
    2.12    static void remember_new_clds(bool remember) { _saved_head = (remember ? _head : NULL); }
    2.13    static GrowableArray<ClassLoaderData*>* new_clds();
    2.14  
    2.15 +  static void set_should_purge(bool b) { _should_purge = b; }
    2.16 +  static void purge_if_needed() {
    2.17 +    // Only purge the CLDG for CMS if concurrent sweep is complete.
    2.18 +    if (_should_purge) {
    2.19 +      purge();
    2.20 +      // reset for next time.
    2.21 +      set_should_purge(false);
    2.22 +    }
    2.23 +  }
    2.24 +
    2.25    static void dump_on(outputStream * const out) PRODUCT_RETURN;
    2.26    static void dump() { dump_on(tty); }
    2.27    static void verify();
    2.28  
    2.29 -  // expensive test for pointer in metaspace for debugging
    2.30 -  static bool contains(const void* x);
    2.31  #ifndef PRODUCT
    2.32    static bool contains_loader_data(ClassLoaderData* loader_data);
    2.33  #endif
     3.1 --- a/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp	Thu May 15 09:25:27 2014 -0400
     3.2 +++ b/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp	Thu May 15 18:23:26 2014 -0400
     3.3 @@ -6363,7 +6363,9 @@
     3.4    verify_overflow_empty();
     3.5  
     3.6    if (should_unload_classes()) {
     3.7 -    ClassLoaderDataGraph::purge();
     3.8 +    // Delay purge to the beginning of the next safepoint.  Metaspace::contains
     3.9 +    // requires that the virtual spaces are stable and not deleted.
    3.10 +    ClassLoaderDataGraph::set_should_purge(true);
    3.11    }
    3.12  
    3.13    _intra_sweep_timer.stop();
     4.1 --- a/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp	Thu May 15 09:25:27 2014 -0400
     4.2 +++ b/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp	Thu May 15 18:23:26 2014 -0400
     4.3 @@ -5416,7 +5416,7 @@
     4.4        if (_g1h->is_in_g1_reserved(p)) {
     4.5          _par_scan_state->push_on_queue(p);
     4.6        } else {
     4.7 -        assert(!ClassLoaderDataGraph::contains((address)p),
     4.8 +        assert(!Metaspace::contains((const void*)p),
     4.9                 err_msg("Otherwise need to call _copy_metadata_obj_cl->do_oop(p) "
    4.10                                PTR_FORMAT, p));
    4.11            _copy_non_heap_obj_cl->do_oop(p);
     5.1 --- a/src/share/vm/memory/allocation.cpp	Thu May 15 09:25:27 2014 -0400
     5.2 +++ b/src/share/vm/memory/allocation.cpp	Thu May 15 18:23:26 2014 -0400
     5.3 @@ -75,7 +75,7 @@
     5.4  }
     5.5  
     5.6  bool MetaspaceObj::is_metaspace_object() const {
     5.7 -  return ClassLoaderDataGraph::contains((void*)this);
     5.8 +  return Metaspace::contains((void*)this);
     5.9  }
    5.10  
    5.11  void MetaspaceObj::print_address_on(outputStream* st) const {
     6.1 --- a/src/share/vm/memory/metaspace.cpp	Thu May 15 09:25:27 2014 -0400
     6.2 +++ b/src/share/vm/memory/metaspace.cpp	Thu May 15 18:23:26 2014 -0400
     6.3 @@ -314,6 +314,8 @@
     6.4    MetaWord* bottom() const { return (MetaWord*) _virtual_space.low(); }
     6.5    MetaWord* end() const { return (MetaWord*) _virtual_space.high(); }
     6.6  
     6.7 +  bool contains(const void* ptr) { return ptr >= low() && ptr < high(); }
     6.8 +
     6.9    size_t reserved_words() const  { return _virtual_space.reserved_size() / BytesPerWord; }
    6.10    size_t committed_words() const { return _virtual_space.actual_committed_size() / BytesPerWord; }
    6.11  
    6.12 @@ -555,6 +557,8 @@
    6.13    void inc_virtual_space_count();
    6.14    void dec_virtual_space_count();
    6.15  
    6.16 +  bool contains(const void* ptr);
    6.17 +
    6.18    // Unlink empty VirtualSpaceNodes and free it.
    6.19    void purge(ChunkManager* chunk_manager);
    6.20  
    6.21 @@ -639,8 +643,6 @@
    6.22    // Accessors
    6.23    Metachunk* chunks_in_use(ChunkIndex index) const { return _chunks_in_use[index]; }
    6.24    void set_chunks_in_use(ChunkIndex index, Metachunk* v) {
    6.25 -    // ensure lock-free iteration sees fully initialized node
    6.26 -    OrderAccess::storestore();
    6.27      _chunks_in_use[index] = v;
    6.28    }
    6.29  
    6.30 @@ -755,8 +757,6 @@
    6.31    void print_on(outputStream* st) const;
    6.32    void locked_print_chunks_in_use_on(outputStream* st) const;
    6.33  
    6.34 -  bool contains(const void *ptr);
    6.35 -
    6.36    void verify();
    6.37    void verify_chunk_size(Metachunk* chunk);
    6.38    NOT_PRODUCT(void mangle_freed_chunks();)
    6.39 @@ -1076,6 +1076,7 @@
    6.40  // nodes with a 0 container_count.  Remove Metachunks in
    6.41  // the node from their respective freelists.
    6.42  void VirtualSpaceList::purge(ChunkManager* chunk_manager) {
    6.43 +  assert(SafepointSynchronize::is_at_safepoint(), "must be called at safepoint for contains to work");
    6.44    assert_lock_strong(SpaceManager::expand_lock());
    6.45    // Don't use a VirtualSpaceListIterator because this
    6.46    // list is being changed and a straightforward use of an iterator is not safe.
    6.47 @@ -1109,8 +1110,8 @@
    6.48    }
    6.49  #ifdef ASSERT
    6.50    if (purged_vsl != NULL) {
    6.51 -  // List should be stable enough to use an iterator here.
    6.52 -  VirtualSpaceListIterator iter(virtual_space_list());
    6.53 +    // List should be stable enough to use an iterator here.
    6.54 +    VirtualSpaceListIterator iter(virtual_space_list());
    6.55      while (iter.repeat()) {
    6.56        VirtualSpaceNode* vsl = iter.get_next();
    6.57        assert(vsl != purged_vsl, "Purge of vsl failed");
    6.58 @@ -1119,6 +1120,23 @@
    6.59  #endif
    6.60  }
    6.61  
    6.62 +
    6.63 +// This function looks at the mmap regions in the metaspace without locking.
    6.64 +// The chunks are added with store ordering and not deleted except for at
    6.65 +// unloading time during a safepoint.
    6.66 +bool VirtualSpaceList::contains(const void* ptr) {
    6.67 +  // List should be stable enough to use an iterator here because removing virtual
    6.68 +  // space nodes is only allowed at a safepoint.
    6.69 +  VirtualSpaceListIterator iter(virtual_space_list());
    6.70 +  while (iter.repeat()) {
    6.71 +    VirtualSpaceNode* vsn = iter.get_next();
    6.72 +    if (vsn->contains(ptr)) {
    6.73 +      return true;
    6.74 +    }
    6.75 +  }
    6.76 +  return false;
    6.77 +}
    6.78 +
    6.79  void VirtualSpaceList::retire_current_virtual_space() {
    6.80    assert_lock_strong(SpaceManager::expand_lock());
    6.81  
    6.82 @@ -1208,6 +1226,8 @@
    6.83    } else {
    6.84      assert(new_entry->reserved_words() == vs_word_size,
    6.85          "Reserved memory size differs from requested memory size");
    6.86 +    // ensure lock-free iteration sees fully initialized node
    6.87 +    OrderAccess::storestore();
    6.88      link_vs(new_entry);
    6.89      return true;
    6.90    }
    6.91 @@ -2431,21 +2451,6 @@
    6.92    return result;
    6.93  }
    6.94  
    6.95 -// This function looks at the chunks in the metaspace without locking.
    6.96 -// The chunks are added with store ordering and not deleted except for at
    6.97 -// unloading time.
    6.98 -bool SpaceManager::contains(const void *ptr) {
    6.99 -  for (ChunkIndex i = ZeroIndex; i < NumberOfInUseLists; i = next_chunk_index(i))
   6.100 -  {
   6.101 -    Metachunk* curr = chunks_in_use(i);
   6.102 -    while (curr != NULL) {
   6.103 -      if (curr->contains(ptr)) return true;
   6.104 -      curr = curr->next();
   6.105 -    }
   6.106 -  }
   6.107 -  return false;
   6.108 -}
   6.109 -
   6.110  void SpaceManager::verify() {
   6.111    // If there are blocks in the dictionary, then
   6.112    // verfication of chunks does not work since
   6.113 @@ -3550,11 +3555,15 @@
   6.114  }
   6.115  
   6.116  bool Metaspace::contains(const void* ptr) {
   6.117 -  if (vsm()->contains(ptr)) return true;
   6.118 -  if (using_class_space()) {
   6.119 -    return class_vsm()->contains(ptr);
   6.120 +  if (UseSharedSpaces && MetaspaceShared::is_in_shared_space(ptr)) {
   6.121 +    return true;
   6.122    }
   6.123 -  return false;
   6.124 +
   6.125 +  if (using_class_space() && get_space_list(ClassType)->contains(ptr)) {
   6.126 +     return true;
   6.127 +  }
   6.128 +
   6.129 +  return get_space_list(NonClassType)->contains(ptr);
   6.130  }
   6.131  
   6.132  void Metaspace::verify() {
   6.133 @@ -3799,5 +3808,4 @@
   6.134    TestVirtualSpaceNodeTest::test();
   6.135    TestVirtualSpaceNodeTest::test_is_available();
   6.136  }
   6.137 -
   6.138  #endif
     7.1 --- a/src/share/vm/memory/metaspace.hpp	Thu May 15 09:25:27 2014 -0400
     7.2 +++ b/src/share/vm/memory/metaspace.hpp	Thu May 15 18:23:26 2014 -0400
     7.3 @@ -1,5 +1,5 @@
     7.4  /*
     7.5 - * Copyright (c) 2011, 2013, Oracle and/or its affiliates. All rights reserved.
     7.6 + * Copyright (c) 2011, 2014, Oracle and/or its affiliates. All rights reserved.
     7.7   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
     7.8   *
     7.9   * This code is free software; you can redistribute it and/or modify it
    7.10 @@ -232,7 +232,8 @@
    7.11    MetaWord* expand_and_allocate(size_t size,
    7.12                                  MetadataType mdtype);
    7.13  
    7.14 -  bool contains(const void* ptr);
    7.15 +  static bool contains(const void* ptr);
    7.16 +
    7.17    void dump(outputStream* const out) const;
    7.18  
    7.19    // Free empty virtualspaces
     8.1 --- a/src/share/vm/oops/klass.cpp	Thu May 15 09:25:27 2014 -0400
     8.2 +++ b/src/share/vm/oops/klass.cpp	Thu May 15 18:23:26 2014 -0400
     8.3 @@ -648,7 +648,7 @@
     8.4  
     8.5    // This can be expensive, but it is worth checking that this klass is actually
     8.6    // in the CLD graph but not in production.
     8.7 -  assert(ClassLoaderDataGraph::contains((address)this), "Should be");
     8.8 +  assert(Metaspace::contains((address)this), "Should be");
     8.9  
    8.10    guarantee(this->is_klass(),"should be klass");
    8.11  
     9.1 --- a/src/share/vm/oops/method.cpp	Thu May 15 09:25:27 2014 -0400
     9.2 +++ b/src/share/vm/oops/method.cpp	Thu May 15 18:23:26 2014 -0400
     9.3 @@ -1873,6 +1873,14 @@
     9.4    loader_data->jmethod_ids()->clear_all_methods();
     9.5  }
     9.6  
     9.7 +bool Method::has_method_vptr(const void* ptr) {
     9.8 +  Method m;
     9.9 +  // This assumes that the vtbl pointer is the first word of a C++ object.
    9.10 +  // This assumption is also in universe.cpp patch_klass_vtble
    9.11 +  void* vtbl2 = dereference_vptr((const void*)&m);
    9.12 +  void* this_vtbl = dereference_vptr(ptr);
    9.13 +  return vtbl2 == this_vtbl;
    9.14 +}
    9.15  
    9.16  // Check that this pointer is valid by checking that the vtbl pointer matches
    9.17  bool Method::is_valid_method() const {
    9.18 @@ -1881,12 +1889,7 @@
    9.19    } else if (!is_metaspace_object()) {
    9.20      return false;
    9.21    } else {
    9.22 -    Method m;
    9.23 -    // This assumes that the vtbl pointer is the first word of a C++ object.
    9.24 -    // This assumption is also in universe.cpp patch_klass_vtble
    9.25 -    void* vtbl2 = dereference_vptr((void*)&m);
    9.26 -    void* this_vtbl = dereference_vptr((void*)this);
    9.27 -    return vtbl2 == this_vtbl;
    9.28 +    return has_method_vptr((const void*)this);
    9.29    }
    9.30  }
    9.31  
    10.1 --- a/src/share/vm/oops/method.hpp	Thu May 15 09:25:27 2014 -0400
    10.2 +++ b/src/share/vm/oops/method.hpp	Thu May 15 18:23:26 2014 -0400
    10.3 @@ -868,6 +868,7 @@
    10.4    const char* internal_name() const { return "{method}"; }
    10.5  
    10.6    // Check for valid method pointer
    10.7 +  static bool has_method_vptr(const void* ptr);
    10.8    bool is_valid_method() const;
    10.9  
   10.10    // Verify
    11.1 --- a/src/share/vm/runtime/os.cpp	Thu May 15 09:25:27 2014 -0400
    11.2 +++ b/src/share/vm/runtime/os.cpp	Thu May 15 18:23:26 2014 -0400
    11.3 @@ -1095,11 +1095,15 @@
    11.4  
    11.5    }
    11.6  
    11.7 -  // Check if in metaspace.
    11.8 -  if (ClassLoaderDataGraph::contains((address)addr)) {
    11.9 -    // Use addr->print() from the debugger instead (not here)
   11.10 -    st->print_cr(INTPTR_FORMAT
   11.11 -                 " is pointing into metadata", addr);
   11.12 +  // Check if in metaspace and print types that have vptrs (only method now)
   11.13 +  if (Metaspace::contains(addr)) {
   11.14 +    if (Method::has_method_vptr((const void*)addr)) {
   11.15 +      ((Method*)addr)->print_value_on(st);
   11.16 +      st->cr();
   11.17 +    } else {
   11.18 +      // Use addr->print() from the debugger instead (not here)
   11.19 +      st->print_cr(INTPTR_FORMAT " is pointing into metadata", addr);
   11.20 +    }
   11.21      return;
   11.22    }
   11.23  
    12.1 --- a/src/share/vm/runtime/safepoint.cpp	Thu May 15 09:25:27 2014 -0400
    12.2 +++ b/src/share/vm/runtime/safepoint.cpp	Thu May 15 18:23:26 2014 -0400
    12.3 @@ -538,6 +538,13 @@
    12.4      gclog_or_tty->rotate_log(false);
    12.5    }
    12.6  
    12.7 +  {
    12.8 +    // CMS delays purging the CLDG until the beginning of the next safepoint and to
    12.9 +    // make sure concurrent sweep is done
   12.10 +    TraceTime t7("purging class loader data graph", TraceSafepointCleanupTime);
   12.11 +    ClassLoaderDataGraph::purge_if_needed();
   12.12 +  }
   12.13 +
   12.14    if (MemTracker::is_on()) {
   12.15      MemTracker::sync();
   12.16    }
    13.1 --- a/src/share/vm/utilities/globalDefinitions.hpp	Thu May 15 09:25:27 2014 -0400
    13.2 +++ b/src/share/vm/utilities/globalDefinitions.hpp	Thu May 15 18:23:26 2014 -0400
    13.3 @@ -1349,7 +1349,7 @@
    13.4  // All C++ compilers that we know of have the vtbl pointer in the first
    13.5  // word.  If there are exceptions, this function needs to be made compiler
    13.6  // specific.
    13.7 -static inline void* dereference_vptr(void* addr) {
    13.8 +static inline void* dereference_vptr(const void* addr) {
    13.9    return *(void**)addr;
   13.10  }
   13.11  

mercurial