Re: RFR: 8289230: Move PlatformXXX class declarations out of os_xxx.hpp [v4]

2022-07-02 Thread kristylee88
On Sat, 2 Jul 2022 04:26:48 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 d

Re: RFR: 8289230: Move PlatformXXX class declarations out of os_xxx.hpp [v4]

2022-07-02 Thread kristylee88
On Sat, 2 Jul 2022 04:26:48 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 d

Re: RFR: 8289230: Move PlatformXXX class declarations out of os_xxx.hpp [v2]

2022-07-02 Thread Ioi Lam
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

Re: RFR: 8289230: Move PlatformXXX class declarations out of os_xxx.hpp [v4]

2022-07-01 Thread Ioi Lam
> 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

Re: RFR: 8289230: Move PlatformXXX class declarations out of os_xxx.hpp [v3]

2022-07-01 Thread Calvin Cheung
On Tue, 28 Jun 2022 20:13:01 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

Re: RFR: 8289230: Move PlatformXXX class declarations out of os_xxx.hpp [v2]

2022-06-28 Thread David Holmes
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

Re: RFR: 8289230: Move PlatformXXX class declarations out of os_xxx.hpp [v2]

2022-06-28 Thread Ioi Lam
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)`

Re: RFR: 8289230: Move PlatformXXX class declarations out of os_xxx.hpp [v2]

2022-06-28 Thread Coleen Phillimore
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)

Re: RFR: 8289230: Move PlatformXXX class declarations out of os_xxx.hpp [v2]

2022-06-28 Thread Ioi Lam
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

Re: RFR: 8289230: Move PlatformXXX class declarations out of os_xxx.hpp [v3]

2022-06-28 Thread Ioi Lam
> 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

Re: RFR: 8289230: Move PlatformXXX class declarations out of os_xxx.hpp [v2]

2022-06-28 Thread Coleen Phillimore
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

Re: RFR: 8289230: Move PlatformXXX class declarations out of os_xxx.hpp [v2]

2022-06-28 Thread Ioi Lam
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

Re: RFR: 8289230: Move PlatformXXX class declarations out of os_xxx.hpp [v2]

2022-06-28 Thread Ioi Lam
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

Re: RFR: 8289230: Move PlatformXXX class declarations out of os_xxx.hpp [v2]

2022-06-28 Thread Ioi Lam
> 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

Re: RFR: 8289230: Move PlatformXXX class declarations out of os_xxx.hpp

2022-06-28 Thread David Holmes
On Tue, 28 Jun 2022 06:16:21 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 declared i

RFR: 8289230: Move PlatformXXX class declarations out of os_xxx.hpp

2022-06-27 Thread Ioi Lam
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 the large header file