RFR: 8248817: Windows: Improving common cross-platform code

[Ludovic Henry]

Hi David, Kim,

Thanks again for pushing that and sorry for the confusion.

> Just in general, I think it's better to split unrelated changes like this,
> rather than having an omnibus bug/webrev.  The changes were split into
> multiple webrevs, but all under the same bug, which led to the confusion
> about who had reviewed what.

Definitely lesson learned for me. I'll make sure to have 1 Webrev for 1 JBS issue in the future.

-----Original Message-----
From: David Holmes <david.holmes at oracle.com> 
Sent: Thursday, July 30, 2020 5:21 AM
To: Kim Barrett <kim.barrett at oracle.com>
Cc: Ludovic Henry <luhenry at microsoft.com>; hotspot-runtime-dev at openjdk.java.net; openjdk-aarch64 <openjdk-aarch64 at microsoft.com>
Subject: Re: RFR: 8248817: Windows: Improving common cross-platform code

On 30/07/2020 9:54 pm, Kim Barrett wrote:
>> On Jul 30, 2020, at 2:56 AM, David Holmes <david.holmes at oracle.com> wrote:
>>
>> Changeset: c35fba4bce35
>> Author:    dholmes
>> Date:      2020-07-30 02:47 -0400
>> URL:       https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fhg.openjdk.java.net%2Fjdk%2Fjdk%2Frev%2Fc35fba4bce35&data=02%7C01%7Cluhenry%40microsoft.com%7C6a4bf40eaf5241578f4508d834834688%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637317085726171698&sdata=pZPT6eCfohM%2F0O2INcW5Dvjdin%2BHP39Nzx%2FpbB3eQA4%3D&reserved=0
>>
>> 8250810: Push missing parts of JDK-8248817
>> Summary: Push changes from JDK-8248817 that were accidentally excluded from the commit.
>> Reviewed-by: kbarrett, dholmes
>> Contributed-by: Ludovic Henry <luhenry at microsoft.com>
>>
>> ! src/hotspot/os/windows/os_windows.cpp
>> ! src/hotspot/os_cpu/windows_x86/thread_windows_x86.cpp
> 
> For the record, I only reviewed the atomics changes (I always referred to
> that change with my comments and approval; sorry that apparently wasn't
> clear), so I think the other two parts (pushed under JDK-8250810) were only
> reviewed by David. Not a disaster, but potentially an oops.

My fault. With the time lag I overlooked the split with the initial 
commit and I didn't then go back and re-read the email thread to check 
the details.

David
-----

> Just in general, I think it's better to split unrelated changes like this,
> rather than having an omnibus bug/webrev.  The changes were split into
> multiple webrevs, but all under the same bug, which led to the confusion
> about who had reviewed what.
> 
>> On 30/07/2020 4:40 pm, David Holmes wrote:
>>> Hi Ludovic,
>>> On 30/07/2020 11:45 am, Ludovic Henry wrote:
>>>> Hi David,
>>>>
>>>> Looking at the change pushed [1], it seems like you pushed only part of the whole change (the bits related to atomics). It's missing the other two webrevs linked in the first email ([2] and [3]). What would be the best way to proceed? Would you like me to submit another review?
>>> Aaarghhh! Sorry. I'll file a new bug and push the other bits under it.
>>> David
>>>> Thanks,
>>>> Ludovic
>>>>
>>>> [1] https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fhg.openjdk.java.net%2Fjdk%2Fjdk%2Frev%2Fbda65def14de&data=02%7C01%7Cluhenry%40microsoft.com%7C6a4bf40eaf5241578f4508d834834688%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637317085726171698&sdata=gc3oth1zAxuAS04lGHPKs6GG6hFB5JstVNM%2BhRkB8kM%3D&reserved=0
>>>> [2] https://nam06.safelinks.protection.outlook.com/?url=http:%2F%2Fcr.openjdk.java.net%2F~burban%2Fluhenry%2F8248817-exception-handling%2F&data=02%7C01%7Cluhenry%40microsoft.com%7C6a4bf40eaf5241578f4508d834834688%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637317085726171698&sdata=WitCgwZwEHYk0vADDXP4B9UwokzmspSYKR2ooaE%2BQmE%3D&reserved=0
>>>> [3] https://nam06.safelinks.protection.outlook.com/?url=http:%2F%2Fcr.openjdk.java.net%2F~burban%2Fluhenry%2F8248817-frames%2F&data=02%7C01%7Cluhenry%40microsoft.com%7C6a4bf40eaf5241578f4508d834834688%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637317085726181691&sdata=gPQ0owTq3ufqJFLHKkN6xK78rgVYy031a7C03EEYq3M%3D&reserved=0
> 
>