RFR: 8225779: Remove unused CollectedHeap::cell_header_size()

[Per Liden]

Here's an updated webrev, which just removes the cell_header_size() and 
its uses, but otherwise keeps the code as is.

http://cr.openjdk.java.net/~pliden/8225779/webrev.1

cheers,
Per

On 6/14/19 1:43 PM, Per Liden wrote:
> Hi,
> 
> On 6/14/19 11:26 AM, Erik Österlund wrote:
>> Hi Per,
>>
>> I don't think it is a good idea to completely revert the changes made 
>> to MacroAssembler::needs_explicit_null_check(). This was not just done 
>> for interfacing. The code we had before was actually wrong.
>>
>> We called that function with both addresses (from signal handlers) and 
>> offsets (from the compiler), and tried to look at the passed in value 
>> whether it quacks like an offset or an address based on its value. If 
>> it looked like an address it would first be normalized to look like an 
>> offset. But it was possible that a much-larger-than-page offset is 
>> passed in that actually looks like an address according to the address 
>> quacking code, which turns the supposedly address looking value into 
>> an offset normalized form that looks like it wouldn't need explicit 
>> null checks because in that form the offset is within one page, while 
>> the original offset passed in totally would need an explicit null 
>> check really, because it totally wasn't within that one page offset we 
>> allowed.
>>
>> To make this more sound, the function was split into two - one used 
>> for addresses passed in by signal handlers, and one used for offsets 
>> used by the compiler. This completely removed the value based check to 
>> determine if the argument looks like an address or offset. I would 
>> definitely like to keep that structure going forward, because 
>> otherwise that bug will be re-introduced.
> 
> I see. I'll revise the patch.
> 
> Thanks!
> Per
> 
>>
>> Otherwise, I do encourage removing the now unnecessary cell header 
>> size abstraction.
>>
>> /Erik
>>
>> On 2019-06-14 08:29, Per Liden wrote:
>>> CollectedHeap::cell_header_size() is no longer used and can be 
>>> removed. This patch is a hg backout of [1] where it was introduced, 
>>> plus the two followup bug fixes [2] & [3].
>>>
>>> This is a clean backout, with the exception of light editing needed 
>>> in two places, because:
>>>
>>> 1) The function Universe::narrow_oop_base() has since been renamed to 
>>> CompressedOops::base().
>>> 2) The include guard CPU_ARM_VM_MACROASSEMBLER_ARM_HPP has since been 
>>> renamed to CPU_ARM_MACROASSEMBLER_ARM_HPP.
>>>
>>> Testing: Tier1-3 on all Oracle platforms
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8225779
>>> Webrev: http://cr.openjdk.java.net/~pliden/8225779/webrev.0
>>>
>>> /Per
>>>
>>> [1] http://hg.openjdk.java.net/jdk/jdk/rev/4ad404da0088
>>> [2] http://hg.openjdk.java.net/jdk/jdk/rev/f642ede2eb07
>>> [3] http://hg.openjdk.java.net/jdk/jdk/rev/18bd95c0e463