RFR: 8225779: Remove unused CollectedHeap::cell_header_size()
Per Liden
per.liden at oracle.com
Mon Jun 17 20:40:39 UTC 2019
Thanks Roman!
/Per
On 6/17/19 10:24 PM, Roman Kennke wrote:
>> 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
>
> Looks good! Thanks!
> Roman
>
>
>>
>> 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
>
More information about the hotspot-dev
mailing list