On 5/7/20 1:35 AM, Mikael Vidstedt wrote:
New webrev here:

webrev: 
http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.01/hotspot/open/webrev/

This pretty much says it all:

> Summary of changes:    90904 lines changed: 8 ins; 90725 del; 171 mod; 103780 unchg


My review is focused on looking at the changes and not looking for missed
changes. I figure there's enough work here just looking at the changes to
keep me occupied for a while and enough people have posted comments about
finding other things to be examined, etc...

Unlike my normal reviews, I won't be listing all the touched files;
(there's _only_ 427 of them...)

Don't forget to make a copyright year update pass before you push.

src/hotspot/os/posix/os_posix.hpp
        L174
    old L175 #ifndef SOLARIS
        L176
       nit - on most of this style of deletion you also got rid of
       one of the blank lines, but not here.

src/hotspot/share/utilities/dtrace.hpp
    old L42: #elif defined(__APPLE__)
    old L44: #include <sys/types.h>
    old L45: #else
    new L32: #include <sys/types.h>
        <sys/types.h> was previous included only for __APPLE__ and it
        is now there for every platform. Any particular reason?

Thumbs up!

Dan


incremental: 
http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.01/hotspot.incr/open/webrev/

Remaining items:

* File follow-up to remove STACK_BIAS

* File follow-ups to change/update/remove flags and/or flag documentation: 
UseLWPSynchronization, BranchOnRegister, LIRFillDelaySlots, 
ArrayAllocatorMallocLimit, ThreadPriorityPolicy

* File follow-up(s) to update comments ("solaris", “sparc”, “solstudio”, 
“sunos”, “sun studio”, “s compiler bug”, “niagara”, …)


Please let me know if there’s something I have missed!

Cheers,
Mikael

On May 3, 2020, at 10:12 PM, Mikael Vidstedt <mikael.vidst...@oracle.com> wrote:


Please review this change which implements part of JEP 381:

JBS: https://bugs.openjdk.java.net/browse/JDK-8244224
webrev: 
http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.00/hotspot/open/webrev/
JEP: https://bugs.openjdk.java.net/browse/JDK-8241787


Note: When reviewing this, please be aware that this exercise was *extremely* 
mind-numbing, so I appreciate your help reviewing all the individual changes 
carefully. You may want to get that coffee cup filled up (or whatever keeps you 
awake)!


Background:

Because of the size of the total patch and wide range of areas touched, this 
patch is one out of in total six partial patches which together make up the 
necessary changes to remove the Solaris and SPARC ports. The other patches are 
being sent out for review to mailing lists appropriate for the respective areas 
the touch. An email will be sent to jdk-dev summarizing all the 
patches/reviews. To be clear: this patch is *not* in itself complete and 
stand-alone - all of the (six) patches are needed to form a complete patch. 
Some changes in this patch may look wrong or incomplete unless also looking at 
the corresponding changes in other areas.

For convenience, I’m including a link below[1] to the full webrev, but in case 
you have comments on changes in other areas, outside of the files included in 
this thread, please provide those comments directly in the thread on the 
appropriate mailing list for that area if possible.

In case it helps, the changes were effectively produced by searching for and 
updating any code mentioning “solaris", “sparc”, “solstudio”, “sunos”, etc. 
More information about the areas impacted can be found in the JEP itself.

A big thank you to Igor Ignatyev for helping make the changes to the hotspot 
tests!

Also, I have a short list of follow-ups which I’m going to look at separately 
from this JEP/patch, mainly related to command line options/flags which are no 
longer relevant and should be deprecated/obsoleted/removed.

Testing:

A slightly earlier version of this change successfully passed tier1-8, as well 
as client tier1-2. Additional testing will be done after the first round of 
reviews has been completed.

Cheers,
Mikael

[1] 
http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.00/all/open/webrev/


Reply via email to