> On May 8, 2020, at 12:48 PM, Daniel D. Daugherty
> <daniel.daughe...@oracle.com> wrote:
>
> 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.
Yup - I have added it in 10 different places on my TODO list to maximize the
likelihood of me remembering it :)
>
> 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.
Oops, will fix.
> 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?
No particular reason other than "it looks cleaner". I guess we could see if the
include can be removed altogether.
> Thumbs up!
Thanks for the review!!
Cheers,
Mikael
>
>> 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/
>>>
>
