On Tue, 28 Jun 2022 19:39:29 GMT, Coleen Phillimore wrote:
>> Ioi Lam has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> fixed comments
>
> I very much like this incremental approach to reducing our over-inclusion.
Thanks @coleenp @calvinc
On Tue, 28 Jun 2022 22:44:55 GMT, Ioi Lam wrote:
>> Doesn't the OS_HEADER() -similar macro that handles the posix case too? I
>> don't see it though.
>
> ` OS_HEADER(threadCrashProtection)` will give
> threadCrashProtection_linux.hpp, etc. It won't give us
> threadCrashProtection_posix.hpp. I
On Tue, 28 Jun 2022 21:02:53 GMT, Coleen Phillimore wrote:
>> I fixed it as you suggested. I also fixed semaphore.hpp where I copied the
>> old pattern from.
>
> Doesn't the OS_HEADER() -similar macro that handles the posix case too? I
> don't see it though.
` OS_HEADER(threadCrashProtection)`
On Tue, 28 Jun 2022 20:08:18 GMT, Ioi Lam wrote:
>> src/hotspot/share/runtime/threadCrashProtection.hpp line 42:
>>
>>> 40: #else
>>> 41: # error "No ThreadCrashProtection implementation provided for this OS"
>>> 42: #endif
>>
>> Shouldn't you use this?
>> #define OS_HEADER(basename)
On Tue, 28 Jun 2022 19:37:01 GMT, Coleen Phillimore wrote:
>> Ioi Lam has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> fixed comments
>
> src/hotspot/share/runtime/mutex.hpp line 203:
>
>> 201: #ifndef PRODUCT
>> 202: void print_on
On Tue, 28 Jun 2022 18:38:10 GMT, Ioi Lam wrote:
>> There are only two implementations of these classes (one for windows, and
>> one for posix):
>>
>> - PlatformEvent
>> - PlatformParker
>> - PlatformMutex
>> - PlatformMonitor
>> - ThreadCrashProtection
>>
>> Before this PR, these classes are
On Tue, 28 Jun 2022 07:20:59 GMT, David Holmes wrote:
> Just took a quick skim through to get the general sense of things. Header
> file split is okay. Pity about the .cpp situation though - maybe move to
> platform_posix.cpp and platform_windows.cpp to at least get them out of the
> os_xxx.cp
On Tue, 28 Jun 2022 07:15:49 GMT, David Holmes wrote:
>> Ioi Lam has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> fixed comments
>
> src/hotspot/os/posix/mutex_posix.hpp line 141:
>
>> 139: };
>> 140:
>> 141: #endif // OS_POSIX_PARK_POS
> There are only two implementations of these classes (one for windows, and one
> for posix):
>
> - PlatformEvent
> - PlatformParker
> - PlatformMutex
> - PlatformMonitor
> - ThreadCrashProtection
>
> Before this PR, these classes are declared in os_xxx.hpp. This causes
> excessive inclusion of
