RFR: 8289230: Move PlatformXXX class declarations out of os_xxx.hpp [v2]
Coleen Phillimore
coleenp at openjdk.org
Tue Jun 28 19:42:44 UTC 2022
On Tue, 28 Jun 2022 18:38:10 GMT, Ioi Lam <iklam at openjdk.org> 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 in os_xxx.hpp. This causes excessive inclusion of the large header file os.hpp by popular headers such as mutex.hpp, which needs only the declaration of PlatformMutex but not the other stuff in os.hpp
>>
>> This PR moves the declarations to park_posix.hpp, mutex_posix.hpp, etc.
>>
>> Note: ideally, the definition of PlatformParker/PlatformEvent should be moved to park_posix.cpp, and PlatformMutex/PlatformMonitor should be moved to mutex_posix.cpp. However, the definition of these 4 classes are intertwined, so I'll leave them inside os_posix.cpp for now. (Same for the Windows version).
>
> 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.
src/hotspot/share/runtime/mutex.hpp line 203:
> 201: #ifndef PRODUCT
> 202: void print_on(outputStream* st) const;
> 203: void print() const { /*print_on(::tty); */ } // FIXME
Can you move this print implementation into the .cpp file?
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) XSTR(OS_HEADER_STEM(basename).hpp)
-------------
Marked as reviewed by coleenp (Reviewer).
PR: https://git.openjdk.org/jdk/pull/9303
More information about the serviceability-dev
mailing list