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