RFR (S/M): 8149127: Rename g1/concurrentMarkThread.* to g1/g1ConcurrentMarkThread.*

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR (S/M): 8149127: Rename g1/concurrentMarkThread.* to g1/g1ConcurrentMarkThread.*

Thomas Schatzl
Hi all,

  can I have reviews for this renaming work as indicated by the
subject?

For easier review I provided three webrevs (although the "full" is not
that hard to review), the first renaming the files, the second renaming
the classes on top of that, and the third the complete webrev I intend
to push.

CR:
https://bugs.openjdk.java.net/browse/JDK-8149127
Webrev:
http://cr.openjdk.java.net/~tschatzl/8149127/webrev/ (full)
http://cr.openjdk.java.net/~tschatzl/8149127/webrev.a/ (file renaming)
http://cr.openjdk.java.net/~tschatzl/8149127/webrev.b/ (class renaming)
Testing:
jprt builds

Thanks,
  Thomas


Reply | Threaded
Open this post in threaded view
|

Re: RFR (S/M): 8149127: Rename g1/concurrentMarkThread.* to g1/g1ConcurrentMarkThread.*

Stefan Johansson
Hi Thomas,

On 2017-11-02 14:03, Thomas Schatzl wrote:

> Hi all,
>
>    can I have reviews for this renaming work as indicated by the
> subject?
>
> For easier review I provided three webrevs (although the "full" is not
> that hard to review), the first renaming the files, the second renaming
> the classes on top of that, and the third the complete webrev I intend
> to push.
>
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8149127
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8149127/webrev/ (full)
> http://cr.openjdk.java.net/~tschatzl/8149127/webrev.a/ (file renaming)
> http://cr.openjdk.java.net/~tschatzl/8149127/webrev.b/ (class renaming)
Thanks for doing this cleanup Thomas.

Looks really good. Just one small thing, could you rename the getter in
G1CollectedHeap:
1392   G1ConcurrentRefine* concurrent_g1_refine() const { return _cg1r; }

I think it would make sense to rename it to g1_concurrent_refine().

Thanks,
Stefan

> Testing:
> jprt builds
>
> Thanks,
>    Thomas
>
>

Reply | Threaded
Open this post in threaded view
|

Re: RFR (S/M): 8149127: Rename g1/concurrentMarkThread.* to g1/g1ConcurrentMarkThread.*

Thomas Schatzl
Hi Stefan,

   thanks for your review.

On Fri, 2017-11-03 at 12:30 +0100, Stefan Johansson wrote:

> Hi Thomas,
>
> On 2017-11-02 14:03, Thomas Schatzl wrote:
> > Hi all,
> >
> >    can I have reviews for this renaming work as indicated by the
> > subject?
> >
> > For easier review I provided three webrevs (although the "full" is
> > not
> > that hard to review), the first renaming the files, the second
> > renaming
> > the classes on top of that, and the third the complete webrev I
> > intend
> > to push.
> >
> > CR:
> > https://bugs.openjdk.java.net/browse/JDK-8149127
> > Webrev:
> > http://cr.openjdk.java.net/~tschatzl/8149127/webrev/ (full)
> > http://cr.openjdk.java.net/~tschatzl/8149127/webrev.a/ (file
> > renaming)
> > http://cr.openjdk.java.net/~tschatzl/8149127/webrev.b/ (class
> > renaming)
>
> Thanks for doing this cleanup Thomas.
>
> Looks really good. Just one small thing, could you rename the getter
> in 
> G1CollectedHeap:
> 1392   G1ConcurrentRefine* concurrent_g1_refine() const { return
> _cg1r; }
>
> I think it would make sense to rename it to g1_concurrent_refine().
>

  renamed to concurrent_refine() - also renamed the few "cg1r" names
floating around for consistency.

http://cr.openjdk.java.net/~tschatzl/8149127/webrev.1/ (full)
http://cr.openjdk.java.net/~tschatzl/8149127/webrev.b.1 (which also
represents the 0_to_1 patch)

Thanks,
  Thomas

Reply | Threaded
Open this post in threaded view
|

Re: RFR (S/M): 8149127: Rename g1/concurrentMarkThread.* to g1/g1ConcurrentMarkThread.*

sangheon.kim@oracle.com
Hi Thomas,

On 11/03/2017 06:47 AM, Thomas Schatzl wrote:

> Hi Stefan,
>
>     thanks for your review.
>
> On Fri, 2017-11-03 at 12:30 +0100, Stefan Johansson wrote:
>> Hi Thomas,
>>
>> On 2017-11-02 14:03, Thomas Schatzl wrote:
>>> Hi all,
>>>
>>>     can I have reviews for this renaming work as indicated by the
>>> subject?
>>>
>>> For easier review I provided three webrevs (although the "full" is
>>> not
>>> that hard to review), the first renaming the files, the second
>>> renaming
>>> the classes on top of that, and the third the complete webrev I
>>> intend
>>> to push.
>>>
>>> CR:
>>> https://bugs.openjdk.java.net/browse/JDK-8149127
>>> Webrev:
>>> http://cr.openjdk.java.net/~tschatzl/8149127/webrev/ (full)
>>> http://cr.openjdk.java.net/~tschatzl/8149127/webrev.a/ (file
>>> renaming)
>>> http://cr.openjdk.java.net/~tschatzl/8149127/webrev.b/ (class
>>> renaming)
>> Thanks for doing this cleanup Thomas.
>>
>> Looks really good. Just one small thing, could you rename the getter
>> in
>> G1CollectedHeap:
>> 1392   G1ConcurrentRefine* concurrent_g1_refine() const { return
>> _cg1r; }
>>
>> I think it would make sense to rename it to g1_concurrent_refine().
>>
>    renamed to concurrent_refine() - also renamed the few "cg1r" names
> floating around for consistency.
>
> http://cr.openjdk.java.net/~tschatzl/8149127/webrev.1/ (full)
> http://cr.openjdk.java.net/~tschatzl/8149127/webrev.b.1 (which also
> represents the 0_to_1 patch)
Looks good.
Nice cleanup!

Thanks,
Sangheon


>
> Thanks,
>    Thomas
>

Reply | Threaded
Open this post in threaded view
|

Re: RFR (S/M): 8149127: Rename g1/concurrentMarkThread.* to g1/g1ConcurrentMarkThread.*

Thomas Schatzl
Hi Sangheon, Stefan,

On Fri, 2017-11-03 at 16:20 -0700, sangheon.kim wrote:
> Hi Thomas,
>
> On 11/03/2017 06:47 AM, Thomas Schatzl wrote:
> >
[...]

> > > Looks really good. Just one small thing, could you rename the
> > > getter in G1CollectedHeap:
> > > 1392   G1ConcurrentRefine* concurrent_g1_refine() const { return
> > > _cg1r; }
> > >
> > > I think it would make sense to rename it to
> > > g1_concurrent_refine().
> > >
> >
> >    renamed to concurrent_refine() - also renamed the few "cg1r"
> > names
> > floating around for consistency.
> >
> > http://cr.openjdk.java.net/~tschatzl/8149127/webrev.1/ (full)
> > http://cr.openjdk.java.net/~tschatzl/8149127/webrev.b.1 (which also
> > represents the 0_to_1 patch)
>
> Looks good.
> Nice cleanup!

  thanks for your reviews.

Thomas