On Tue, 28 Jun 2022 06:16:21 GMT, Ioi Lam <ik...@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).
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.cpp file? src/hotspot/os/posix/mutex_posix.hpp line 141: > 139: }; > 140: > 141: #endif // OS_POSIX_PARK_POSIX_HPP Wrong comment. src/hotspot/os/posix/threadCrashProtection_posix.cpp line 38: > 36: > 37: /* > 38: * See the caveats for this class in os_posix.hpp Comment needs updating src/hotspot/os/windows/mutex_windows.hpp line 64: > 62: }; > 63: > 64: #endif // OS_WINDOWS_PARK_WINDOWS_HPP Wrong comment ------------- PR: https://git.openjdk.org/jdk/pull/9303
