Re: RFR: 8313141: Missing check for os_thread type in os_windows.cpp [v2]
os::os_thread is not handled in os_windows.cpp, and therefore ignores the thread stack size option. I am unsure if this is the right way to handle os::os_thread, please let me know if it is not
Julian Waters has updated the pull request incrementally with one additional commit since the last revision: Switch os::os_thread to default behaviour ------------- Changes: - all: https://git.openjdk.org/jdk/pull/15079/files - new: https://git.openjdk.org/jdk/pull/15079/files/9c136e64..bf3cf8ea Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=15079&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=15079&range=00-01 Stats: 3 lines in 1 file changed: 2 ins; 1 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/15079.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15079/head:pull/15079 PR: https://git.openjdk.org/jdk/pull/15079
On Sat, 29 Jul 2023 05:50:18 GMT, Julian Waters <jwaters@openjdk.org> wrote:
os::os_thread is not handled in os_windows.cpp, and therefore ignores the thread stack size option. I am unsure if this is the right way to handle os::os_thread, please let me know if it is not
Julian Waters has updated the pull request incrementally with one additional commit since the last revision:
Switch os::os_thread to default behaviour
src/hotspot/os/windows/os_windows.cpp line 721:
719: case os::asynclog_thread: 720: case os::watcher_thread: 721: case os::os_thread:
How did you determine this is the correct behaviour for this type of thread? This will result in a change in behaviour. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/15079#discussion_r1278255078
On Sat, 29 Jul 2023 05:55:02 GMT, Julian Waters <jwaters@openjdk.org> wrote:
os::os_thread is not handled in os_windows.cpp, and therefore ignores the thread stack size option. I am unsure if this is the right way to handle os::os_thread, please let me know if it is not
Julian Waters has updated the pull request incrementally with one additional commit since the last revision:
Switch os::os_thread to default behaviour
It looks like the other platforms have a default case, instead of an explicit os_thread case, that considers VMThreadStackSize. For example, see os::Posix::get_initial_stack_size(): default: // presume the unknown thr_type is a VM internal product_pd(intx, VMThreadStackSize, "Non-Java Thread Stack Size (in Kbytes)") range(0, max_intx/(1 * K)) Align with how this is done for the *nix platforms? ------------- PR Comment: https://git.openjdk.org/jdk/pull/15079#issuecomment-1658192633
On Sat, 29 Jul 2023 05:55:02 GMT, Julian Waters <jwaters@openjdk.org> wrote:
os::os_thread is not handled in os_windows.cpp, and therefore ignores the thread stack size option. I am unsure if this is the right way to handle os::os_thread, please let me know if it is not
Julian Waters has updated the pull request incrementally with one additional commit since the last revision:
Switch os::os_thread to default behaviour
Sure, I'll add the default to be the same as the rest in that case (no pun intended) ------------- PR Comment: https://git.openjdk.org/jdk/pull/15079#issuecomment-1659650127
participants (3)
-
David Holmes
-
Julian Waters
-
Markus Grönlund