RFR (T) 8243393: Improve ReservedSpace constructor resolution
Thomas Stüfe
thomas.stuefe at gmail.com
Wed Apr 29 13:00:16 UTC 2020
I feel responsible because I opened the bug. I think that ReservedSpace
could do with some better commenting and possible cleaning up, but I think
that would not a five minutes project. Lets fix the obvious bug and leave
the clean up for later?
..Thomas
On Wed, Apr 29, 2020 at 2:53 PM David Holmes <david.holmes at oracle.com>
wrote:
> On 29/04/2020 10:38 pm, coleen.phillimore at oracle.com wrote:
> >
> > I already checked this in because it was trivial.
>
> Didn't seem trivial to me.
>
> > On 4/28/20 11:53 PM, David Holmes wrote:
> >> Hi Coleen,
> >>
> >> On 28/04/2020 2:12 am, coleen.phillimore at oracle.com wrote:
> >>> Summary: Remove possibly ambiguous constructor and use directly in
> >>> ReservedCodeHeap
> >>>
> >>> Tested with tier1-3.
> >>>
> >>> open webrev at
> >>> http://cr.openjdk.java.net/~coleenp/2020/8243393.01/webrev
> >>> bug link https://bugs.openjdk.java.net/browse/JDK-8243393
> >>
> >> Sorry but I don't understand the mapping to the constructors. In this
> >> original call from cardTable.cpp:
> >>
> >> ReservedSpace heap_rs(_byte_map_size, rs_align, false);
> >>
> >> this calls the 4-arg constructor as-if:
> >>
> >> ReservedSpace heap_rs(_byte_map_size, rs_align, false, NULL);
> >>
> >> and rs_align maps to the alignment parameter.
> >>
> >> But in the new two-arg constructor call:
> >>
> >> ReservedSpace heap_rs(_byte_map_size, rs_align)
> >>
> >> the rs_align is no longer treated as alignment but rather
> >> preferred_page_size! It isn't at all obvious to me that this is an
> >> equivalent construction.
> >>
> >
> http://cr.openjdk.java.net/~coleenp/2020/8243393.01/webrev/src/hotspot/share/gc/shared/cardTable.cpp.udiff.html
> >
> >
> > Yes, you're right, this is part of the change is wrong. It was matching
> > the version I took out. I'll file a bug to fix it.
>
> Okay.
>
> >> Both constructors need to be adequately documented - at present only
> >> the two-arg constructor is.
> >
> > Please suggest a comment.
>
> Ha! I don't understand what the args to these constructors are supposed
> to indicate - that is the problem.
>
> Cheers,
> David
> -----
>
> > thanks,
> > Coleen
> >>
> >> Thanks,
> >> David
> >>
> http://cr.openjdk.java.net/~coleenp/2020/8243393.01/webrev/src/hotspot/share/gc/shared/cardTable.cpp.udiff.html
> >>
> >>> Thanks,
> >>> Coleen
> >
>
More information about the hotspot-runtime-dev
mailing list