RFR(S): 8245021: Add method 'remove_if_existing' to growableArray.

[Liu, Xin]

Hi, Patric, 

I don't object to your change.  I feel that the API 'remove' of GrowableArray was not good.  Even though it's complexity is still linear, you scan all elements and write some of them. 

The problem is it has to retain order. Actually, I didn't run into any problem when I replace the removing element with the last one.  
It suggests that probably nobody in hotspot makes use the sorted GrowableArray. 

I found another interesting point.  There's an API delete_at which ignore orders, so I try and replace your remove_if_exists with it. 
  bool delete_if_existing(const E& elem) {
    int index = find(elem);

    if (index != -1) {
      _data[index] = _data[--_len];
      return true;
    }

    return false;
  }
I didn't have any regression in jtreg:hotspot:tier1.  Actually, CodeCache::unregister_old_nmethod use the same trick. 

Here is current implementation of delete_at().  It checks if the index is the last element, and skip copying if so.  I am not sure if an extra comparison is worthy here. 
Users should use pop() instead in that scenario.
  
  // The order is changed.
  void delete_at(int index) {
    assert(0 <= index && index < _len, "illegal index");
    if (index < --_len) {
      // Replace removed element with last one.
      _data[index] = _data[_len];
    }
  }

Thanks,
--lx

On 5/18/20, 1:38 PM, "hotspot-compiler-dev on behalf of Patric Hedlin" <hotspot-compiler-dev-bounces at openjdk.java.net on behalf of patric.hedlin at oracle.com> wrote:

    CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.



    Dear all,

    I would like to ask for help to review the following change/update:

    Issue:  https://bugs.openjdk.java.net/browse/JDK-8245021
    Webrev: http://cr.openjdk.java.net/~phedlin/tr8245021/


    8245021: Add method 'remove_if_existing' to growableArray.

    Minor improvement to simplify the code pattern "if contains then remove"
    found in a few places (in "compile.hpp").


    Testing: hs-tier1-3


    Best regards,
    Patric