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

[coleen.phillimore at oracle.com]

On 2/19/19 3:22 AM, 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.

Yes, can confirm.
>
> 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/

This looks good.

http://cr.openjdk.java.net/~redestad/8219229/open.01/src/hotspot/share/oops/constantPool.hpp.udiff.html

Small nit, can you make this one line:

- constantTag tag_at(int which) const;
+ constantTag tag_at(int which) const {
+ return (constantTag)tags()->at_acquire(which);
+ }


> 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...

Agree, but moving these functions that used OrderAccess functions back 
seems fine to me.

Thanks,
Coleen

>
> Thanks!
>
> /Claes
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8204301