RFR: 8219229: Make ConstantPool::tag_at and release_tag_at_put inlineable

[David Holmes]

Hi Claes,

Updates look good!

I have one small cleanup request in src/hotspot/share/oops/array.hpp. 
Can we make at_acquire and release_at_put consistent with at and at_put, 
both in terms of parameter naming and the layout style ie.

     T    at(int i) const                 { assert... }
     void at_put(const int i, const T& x) { assert... }
     T*   adr_at(const int i)             { assert... }
     int  find(const T& x)                { return... }

     T    at_acquire(const int i)         { return... }
     void release_at_put(int i, T x)      { Order...  }

Which now begs a couple of questions:
- why isn't at_acquire const?
- why does at_put take a T& but release_at_put takes a T?

Thanks,
David
-----

On 19/02/2019 6:22 pm, Claes Redestad wrote:
> Hi David,
> 
> On 2019-02-19 06:52, David Holmes wrote:
>>>
>>> Webrev: http://cr.openjdk.java.net/~redestad/8219229/open.00/
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8219229
>>
>> Can you explain why you moved an inline definition out of 
>> src/hotspot/share/oops/array.inline.hpp into the plain array.hpp file? 
>> That seems the reverse of what we're supposed to be doing. It also 
>> looks very odd to leave release_at_put as the sole method defined in 
>> the .inline.hpp file.
> 
> array.inline.hpp was created before JDK-8204301[1] which moved
> OrderAccess::load_acquire and store_release from the now removed
> orderAccess.inline.hpp to orderAccess.hpp.
> 
> These array methods thus no longer _need_ to be defined in
> array.inline.hpp, and to be able to define tag_at in constantPool.hpp I
> needed Array<T>::at_acquire in array.hpp.
> 
> That said, yes, it seems silly to keep array.inline.hpp around only for
> release_at_put. constantPool is the only user of these methods, so I
> think we might as well move it all to constantPool.hpp and remove
> array.inline.hpp for now, as both methods are quite trivial. This makes
> for an overall cleaner patch:
> 
> http://cr.openjdk.java.net/~redestad/8219229/open.01/
> 
> I agree moving things from .inline.hpp to hpp files seems like taking
> steps in the wrong direction. I tried defining tag_at in
> constantPool.inline.hpp but it kept growing until I lost track:
> tag_at is used everywhere, there are a lot of asserts in
> constantPool.hpp that'd have to be moved or replaced.
> 
> To reduce code in constantPool.hpp it might be easier to start factoring
> out less frequently used methods...
> 
> Thanks!
> 
> /Claes
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8204301