RFR: 8138888: Remove ExtendedOopClosure::apply_to_weak_ref_discovered_field
Kim Barrett
kim.barrett at oracle.com
Wed May 3 18:05:21 UTC 2017
> On May 3, 2017, at 4:47 AM, Stefan Johansson <stefan.johansson at oracle.com> wrote:
>
>
>
> On 2017-05-02 22:41, Kim Barrett wrote:
>>> On May 2, 2017, at 11:12 AM, Stefan Johansson <stefan.johansson at oracle.com> wrote:
>>>
>>> Thanks Kim for looking at this,
>>>
>>> On 2017-04-28 20:34, Kim Barrett wrote:
>>>> src/share/vm/oops/instanceRefKlass.inline.hpp
>>>>
>>>> 114 void InstanceRefKlass::oop_oop_iterate_discovered_and_discovery(oop obj, ReferenceType type, OopClosureType* closure, Contains& contains) {
>>>> 115 // Explicitly apply closure to the discovered field.
>>>> 116 do_discovered<nv, T>(obj, closure, contains);
>>>> 117 // Then do normal reference processing with discovery.
>>>> 118 oop_oop_iterate_discovery<nv, T>(obj, type, closure, contains);
>>>> 119 }
>>>>
>>>> This will apply the closure to the discovered field, and then
>>>> (conditionally) log the processing of the reference (at the start of
>>>> oop_oop_iterate_discovery).
>>>>
>>>> Also, oop_oop_iterate_fields doesn't have a corresponding log message
>>>> at the start.
>>>>
>>>> Maybe the thing to do is to have the on-entry logging in
>>>> oop_oop_iterate_ref_processing_specialized, with the message
>>>> specialized by the iteration mode.
>>> I agree, the logging here could be a bit more unified. What do you think about using trace_reference_gc for all of them? Like this:
>>> Webrev: http://cr.openjdk.java.net/~sjohanss/XXXXXXX/hotspot.00/
>>>
>>> I would like to push that as a separate RFE though.
>> I like this additional change. If it’s going to be a separate RFE then I’d prefer
>> it went first, so there isn’t a (hopefully temporary) stage where things are kind
>> of wrong. I realize that’s more work, so I won’t insist.
> I was a bit unclear. I want to push them together, but as separate changesets.
>
> I created sub-task for the log change and moved the webrev.
> Sub-task: https://bugs.openjdk.java.net/browse/JDK-8179550
> Webrev: http://cr.openjdk.java.net/~sjohanss/8179550/hotspot.00/
>
> Still need a second reviewer for the changes though.
>
> Thanks,
> Stefan
OK. Looks good.
More information about the hotspot-gc-dev
mailing list