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

[Coleen Phillimore]

Hi Stefan,  Thank you for reviewing again.

On 5/15/14, 2:24 AM, Stefan Karlsson wrote:
> Hi Coleen,
>
> On 2014-05-13 02:19, Coleen Phillimore wrote:
>> Summary: Only prune metaspace virtual spaces at safepoint so walking 
>> them is safe outside a safepoint.
>>
>> Walking class loader data graph for contains is really slow for 
>> applications like nashorn that has a lot of class loader data.
>>
>> Tested with nsk.quick.testlist, jtreg tests, and jck tests. Also ran
>>
>> open webrev at http://cr.openjdk.java.net/~coleenp/8038212/
>> bug link https://bugs.openjdk.java.net/browse/JDK-8038212
>
> Thanks for fixing this.
>
> I looked at the version at:
> http://cr.openjdk.java.net/~coleenp/8038212_3
>
> Could you explain why you added the storestore here? Shouldn't it be 
> placed after the node has been initialized but before it is linked 
> into the list?
> *** 1219,1228 ****
> --- 1237,1248 ----
>      if (virtual_space_list() == NULL) {
>          set_virtual_space_list(new_entry);
>      } else {
>        current_virtual_space()->set_next(new_entry);
>      }
> +   // ensure lock-free iteration sees fully initialized node
> +   OrderAccess::storestore();
>      set_current_virtual_space(new_entry);
>      inc_reserved_words(new_entry->reserved_words());
>      inc_committed_words(new_entry->committed_words());
>      inc_virtual_space_count();
>    #ifdef ASSERT

I meant to revert this to where it was before the change to iterate over 
ClassLoaderDataGraph, but forgot where it was.   I have restored it to 
where it was now, here:

@@ -1210,6 +1228,8 @@
    } else {
      assert(new_entry->reserved_words() == vs_word_size,
          "Reserved memory size differs from requested memory size");
+    // ensure lock-free iteration sees fully initialized node
+    OrderAccess::storestore();
      link_vs(new_entry);
      return true;
    }

Thanks for catching that.

Coleen

>
> Otherwise, this looks good.
>
> thanks,
> StefanK
>>
>> Marcus L. tested it with nashorn tests.
>>
>> Thanks,
>> Coleen
>