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

[Roman Kennke]

Just as I sent my approval, I was thinking the same. The new needs_explicit_null_check() is more correct and also better readable.

Roman

Am 14. Juni 2019 11:26:08 MESZ schrieb "Erik Österlund" <erik.osterlund at oracle.com>:
>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.
>
>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

-- 
Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet.