Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v2]
On Tue, 31 May 2022 15:10:38 GMT, Adam Sotona wrote: >> This is continuation of PR #4256 >> >> The patch simply rounds up the specified stack size to multiple of the >> system page size, on systems where necessary. >> The patch is based on the original PR/branch, with reflected remaining >> recommendations. >> >> Please review. >> >> Thank you, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > Updated Java mannpage Looks good. - Marked as reviewed by stuefe (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8953
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v2]
On Tue, 31 May 2022 15:10:38 GMT, Adam Sotona wrote: >> This is continuation of PR #4256 >> >> The patch simply rounds up the specified stack size to multiple of the >> system page size, on systems where necessary. >> The patch is based on the original PR/branch, with reflected remaining >> recommendations. >> >> Please review. >> >> Thank you, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > Updated Java mannpage Thanks for the manpage update! - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8953
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v2]
On Tue, 31 May 2022 13:28:30 GMT, David Holmes wrote: >> Adam Sotona has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Updated Java mannpage > > src/java.base/share/classes/sun/launcher/resources/launcher.properties line > 176: > >> 174: \-Xssset java thread stack size\n\ >> 175: \ The actual size may be rounded up to a multiple >> of the\n\ >> 176: \ system page size as required by the operating >> system.\n\ > > The Java manpage will also need this update. Thanks for reminder, I've update Java manpage at both locations. - PR: https://git.openjdk.java.net/jdk/pull/8953
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v2]
> This is continuation of PR #4256 > > The patch simply rounds up the specified stack size to multiple of the system > page size, on systems where necessary. > The patch is based on the original PR/branch, with reflected remaining > recommendations. > > Please review. > > Thank you, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: Updated Java mannpage - Changes: - all: https://git.openjdk.java.net/jdk/pull/8953/files - new: https://git.openjdk.java.net/jdk/pull/8953/files/d082aed0..7712f700 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8953&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8953&range=00-01 Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/8953.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8953/head:pull/8953 PR: https://git.openjdk.java.net/jdk/pull/8953
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS
On Tue, 31 May 2022 08:37:19 GMT, Adam Sotona wrote: > This is continuation of PR #4256 > > The patch simply rounds up the specified stack size to multiple of the system > page size, on systems where necessary. > The patch is based on the original PR/branch, with reflected remaining > recommendations. > > Please review. > > Thank you, > Adam Changes looks good. But the java manpage needs an update too. Thanks, David src/java.base/share/classes/sun/launcher/resources/launcher.properties line 176: > 174: \-Xssset java thread stack size\n\ > 175: \ The actual size may be rounded up to a multiple > of the\n\ > 176: \ system page size as required by the operating > system.\n\ The Java manpage will also need this update. - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8953
RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS
This is continuation of PR #4256 The patch simply rounds up the specified stack size to multiple of the system page size, on systems where necessary. The patch is based on the original PR/branch, with reflected remaining recommendations. Please review. Thank you, Adam - Commit messages: - puting EINVAL on the RHS of the == as recommended - Update help text - Cast type - Change java -X output for -Xss - Only try to round-up when current value failed - Avoid overflow on page size - JDK-8236569: -Xss not multiple of 4K does not work for the main thread on macOS Changes: https://git.openjdk.java.net/jdk/pull/8953/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8953&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8236569 Stats: 41 lines in 3 files changed: 39 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/8953.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8953/head:pull/8953 PR: https://git.openjdk.java.net/jdk/pull/8953
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v6]
On Tue, 8 Jun 2021 16:53:38 GMT, Henry Jen wrote: >> …d on macOS >> >> This patch simply round up the specified stack size to multiple of the >> system page size. >> >> Test is trivial, simply run java with -Xss option against following code. On >> MacOS, before the fix, running with `-Xss159k` and `-Xss160k` would get >> `7183` and `649` respectively. After fix, both would output `649`, while >> `-Xss161k` would be same as `-Xss164k` and see 691 as the output. >> >> ```code:java >> public class StackLeak { >> public int depth = 0; >> public void stackLeak() { >> depth++; >> stackLeak(); >> } >> >> public static void main(String[] args) { >> var test = new StackLeak(); >> try { >> test.stackLeak(); >> } catch (Throwable e) { >> System.out.println(test.depth); >> } >> } >> } > > Henry Jen has updated the pull request incrementally with one additional > commit since the last revision: > > Update help text Continuation in #8953 - PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v6]
On Tue, 8 Jun 2021 16:53:38 GMT, Henry Jen wrote: >> …d on macOS >> >> This patch simply round up the specified stack size to multiple of the >> system page size. >> >> Test is trivial, simply run java with -Xss option against following code. On >> MacOS, before the fix, running with `-Xss159k` and `-Xss160k` would get >> `7183` and `649` respectively. After fix, both would output `649`, while >> `-Xss161k` would be same as `-Xss164k` and see 691 as the output. >> >> ```code:java >> public class StackLeak { >> public int depth = 0; >> public void stackLeak() { >> depth++; >> stackLeak(); >> } >> >> public static void main(String[] args) { >> var test = new StackLeak(); >> try { >> test.stackLeak(); >> } catch (Throwable e) { >> System.out.println(test.depth); >> } >> } >> } > > Henry Jen has updated the pull request incrementally with one additional > commit since the last revision: > > Update help text Marked as reviewed by alanb (Reviewer). src/java.base/unix/native/libjli/java_md.c line 681: > 679: > 680: if (stack_size > 0) { > 681: if (EINVAL == pthread_attr_setstacksize(&attr, stack_size)) { Style-wise it might be more consistent put EINVAL on the RHS of the ==. - PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v5]
On Tue, 8 Jun 2021 13:37:10 GMT, Thomas Stuefe wrote: >> Henry Jen has updated the pull request with a new target base due to a merge >> or a rebase. The incremental webrev excludes the unrelated changes brought >> in by the merge/rebase. The pull request contains seven additional commits >> since the last revision: >> >> - Cast type >> - Merge >> - Change java -X output for -Xss >> - Merge >> - Only try to round-up when current value failed >> - Avoid overflow on page size >> - JDK-8236569: -Xss not multiple of 4K does not work for the main thread on >> macOS > > Please make sure the failing tests have nothing to do with your patch. > `gc/shenandoah/compiler/TestLinkToNativeRBP.java` > sounds at least suggestive. Still pending CSR, also considering adapt hotspot align up as suggested by @tstuefe. - PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v5]
On Tue, 8 Jun 2021 16:45:12 GMT, Henry Jen wrote: >> src/java.base/unix/native/libjli/java_md.c line 666: >> >>> 664: return page_size * pages; >>> 665: } >>> 666: } >> >> Could probably be shortened to something like this: >> >> >> size_t pagesize = (size_t)sysconf(_SC_PAGESIZE); >> return (stack_size + (pagesize - 1)) & ~(pagesize - 1); >> >> >> or, if you insist on checking for SIZE_MAX: >> >> >> size_t pagesize = (size_t)sysconf(_SC_PAGESIZE); >> size_t max = SIZE_MAX - pagesize; >> return stack_size <= max ? (stack_size + (pagesize - 1)) & ~(pagesize - 1) : >> max; >> >> >> (I see David requested this, so this is fine, though passing SIZE_MAX to >> this function will quite likely fail anyway :) > > While sysconf(_SC_PAGESIZE) is most likely(if not always) be power of 2, it's > not a constant we know for sure here and this is not critical path for > performance, thus I didn't take that approach. My concern was not performance but brevity, especially since you add the same function twice. And about using the same logic for aligning up as we do within hotspot (see align.hpp). You also mix up size_t and long (signed vs unsigned, and potentially different sizes) - there is nothing obvious wrong with that but I would at least consistently use size_t here. - PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v6]
On Tue, 8 Jun 2021 16:53:38 GMT, Henry Jen wrote: >> …d on macOS >> >> This patch simply round up the specified stack size to multiple of the >> system page size. >> >> Test is trivial, simply run java with -Xss option against following code. On >> MacOS, before the fix, running with `-Xss159k` and `-Xss160k` would get >> `7183` and `649` respectively. After fix, both would output `649`, while >> `-Xss161k` would be same as `-Xss164k` and see 691 as the output. >> >> ```code:java >> public class StackLeak { >> public int depth = 0; >> public void stackLeak() { >> depth++; >> stackLeak(); >> } >> >> public static void main(String[] args) { >> var test = new StackLeak(); >> try { >> test.stackLeak(); >> } catch (Throwable e) { >> System.out.println(test.depth); >> } >> } >> } > > Henry Jen has updated the pull request incrementally with one additional > commit since the last revision: > > Update help text Marked as reviewed by dholmes (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v5]
On 8/06/2021 11:40 pm, Thomas Stuefe wrote: On Mon, 7 Jun 2021 03:18:32 GMT, Henry Jen wrote: …d on macOS This patch simply round up the specified stack size to multiple of the system page size. Test is trivial, simply run java with -Xss option against following code. On MacOS, before the fix, running with `-Xss159k` and `-Xss160k` would get `7183` and `649` respectively. After fix, both would output `649`, while `-Xss161k` would be same as `-Xss164k` and see 691 as the output. ```code:java public class StackLeak { public int depth = 0; public void stackLeak() { depth++; stackLeak(); } public static void main(String[] args) { var test = new StackLeak(); try { test.stackLeak(); } catch (Throwable e) { System.out.println(test.depth); } } } Henry Jen has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision: - Cast type - Merge - Change java -X output for -Xss - Merge - Only try to round-up when current value failed - Avoid overflow on page size - JDK-8236569: -Xss not multiple of 4K does not work for the main thread on macOS Please make sure the failing tests have nothing to do with your patch. `gc/shenandoah/compiler/TestLinkToNativeRBP.java` sounds at least suggestive. warning: using incubating module(s): jdk.incubator.foreign /home/runner/work/jdk/jdk/test/hotspot/jtreg/gc/shenandoah/compiler/TestLinkToNativeRBP.java:42: error: cannot find symbol import jdk.incubator.foreign.LibraryLookup; Looks like shenandoah test was not updated for latest Foreign changes. David - PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v6]
> …d on macOS > > This patch simply round up the specified stack size to multiple of the system > page size. > > Test is trivial, simply run java with -Xss option against following code. On > MacOS, before the fix, running with `-Xss159k` and `-Xss160k` would get > `7183` and `649` respectively. After fix, both would output `649`, while > `-Xss161k` would be same as `-Xss164k` and see 691 as the output. > > ```code:java > public class StackLeak { > public int depth = 0; > public void stackLeak() { > depth++; > stackLeak(); > } > > public static void main(String[] args) { > var test = new StackLeak(); > try { > test.stackLeak(); > } catch (Throwable e) { > System.out.println(test.depth); > } > } > } Henry Jen has updated the pull request incrementally with one additional commit since the last revision: Update help text - Changes: - all: https://git.openjdk.java.net/jdk/pull/4256/files - new: https://git.openjdk.java.net/jdk/pull/4256/files/5a8d4a10..a3966612 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4256&range=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4256&range=04-05 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/4256.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4256/head:pull/4256 PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v5]
On Tue, 8 Jun 2021 08:17:54 GMT, Thomas Stuefe wrote: >> Henry Jen has updated the pull request with a new target base due to a merge >> or a rebase. The incremental webrev excludes the unrelated changes brought >> in by the merge/rebase. The pull request contains seven additional commits >> since the last revision: >> >> - Cast type >> - Merge >> - Change java -X output for -Xss >> - Merge >> - Only try to round-up when current value failed >> - Avoid overflow on page size >> - JDK-8236569: -Xss not multiple of 4K does not work for the main thread on >> macOS > > src/java.base/unix/native/libjli/java_md.c line 666: > >> 664: return page_size * pages; >> 665: } >> 666: } > > Could probably be shortened to something like this: > > > size_t pagesize = (size_t)sysconf(_SC_PAGESIZE); > return (stack_size + (pagesize - 1)) & ~(pagesize - 1); > > > or, if you insist on checking for SIZE_MAX: > > > size_t pagesize = (size_t)sysconf(_SC_PAGESIZE); > size_t max = SIZE_MAX - pagesize; > return stack_size <= max ? (stack_size + (pagesize - 1)) & ~(pagesize - 1) : > max; > > > (I see David requested this, so this is fine, though passing SIZE_MAX to this > function will quite likely fail anyway :) While sysconf(_SC_PAGESIZE) is most likely(if not always) be power of 2, it's not a constant we know for sure here and this is not critical path for performance, thus I didn't take that approach. - PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v5]
On Tue, 8 Jun 2021 02:36:26 GMT, David Holmes wrote: >> Henry Jen has updated the pull request with a new target base due to a merge >> or a rebase. The incremental webrev excludes the unrelated changes brought >> in by the merge/rebase. The pull request contains seven additional commits >> since the last revision: >> >> - Cast type >> - Merge >> - Change java -X output for -Xss >> - Merge >> - Only try to round-up when current value failed >> - Avoid overflow on page size >> - JDK-8236569: -Xss not multiple of 4K does not work for the main thread on >> macOS > > src/java.base/share/classes/sun/launcher/resources/launcher.properties line > 175: > >> 173: \ configuration and continue\n\ >> 174: \-Xssset java thread stack size\n\ >> 175: \ The actual size may be round up to multiple of >> system\n\ > > See updated help text in the CSR request. Updated, thanks - PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v5]
On Mon, 7 Jun 2021 03:18:32 GMT, Henry Jen wrote: >> …d on macOS >> >> This patch simply round up the specified stack size to multiple of the >> system page size. >> >> Test is trivial, simply run java with -Xss option against following code. On >> MacOS, before the fix, running with `-Xss159k` and `-Xss160k` would get >> `7183` and `649` respectively. After fix, both would output `649`, while >> `-Xss161k` would be same as `-Xss164k` and see 691 as the output. >> >> ```code:java >> public class StackLeak { >> public int depth = 0; >> public void stackLeak() { >> depth++; >> stackLeak(); >> } >> >> public static void main(String[] args) { >> var test = new StackLeak(); >> try { >> test.stackLeak(); >> } catch (Throwable e) { >> System.out.println(test.depth); >> } >> } >> } > > Henry Jen has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains seven additional commits since > the last revision: > > - Cast type > - Merge > - Change java -X output for -Xss > - Merge > - Only try to round-up when current value failed > - Avoid overflow on page size > - JDK-8236569: -Xss not multiple of 4K does not work for the main thread on > macOS Please make sure the failing tests have nothing to do with your patch. `gc/shenandoah/compiler/TestLinkToNativeRBP.java` sounds at least suggestive. - PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v5]
On Mon, 7 Jun 2021 03:18:32 GMT, Henry Jen wrote: >> …d on macOS >> >> This patch simply round up the specified stack size to multiple of the >> system page size. >> >> Test is trivial, simply run java with -Xss option against following code. On >> MacOS, before the fix, running with `-Xss159k` and `-Xss160k` would get >> `7183` and `649` respectively. After fix, both would output `649`, while >> `-Xss161k` would be same as `-Xss164k` and see 691 as the output. >> >> ```code:java >> public class StackLeak { >> public int depth = 0; >> public void stackLeak() { >> depth++; >> stackLeak(); >> } >> >> public static void main(String[] args) { >> var test = new StackLeak(); >> try { >> test.stackLeak(); >> } catch (Throwable e) { >> System.out.println(test.depth); >> } >> } >> } > > Henry Jen has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains seven additional commits since > the last revision: > > - Cast type > - Merge > - Change java -X output for -Xss > - Merge > - Only try to round-up when current value failed > - Avoid overflow on page size > - JDK-8236569: -Xss not multiple of 4K does not work for the main thread on > macOS Hi, proposed shorter form. Otherwise this looks fine. Cheers, Thomas src/java.base/unix/native/libjli/java_md.c line 666: > 664: return page_size * pages; > 665: } > 666: } Could probably be shortened to something like this: size_t pagesize = (size_t)sysconf(_SC_PAGESIZE); return (stack_size + (pagesize - 1)) & ~(pagesize - 1); or, if you insist on checking for SIZE_MAX: size_t pagesize = (size_t)sysconf(_SC_PAGESIZE); size_t max = SIZE_MAX - pagesize; return stack_size <= max ? (stack_size + (pagesize - 1)) & ~(pagesize - 1) : max; (I see David requested this, so this is fine, though passing SIZE_MAX to this function will quite likely fail anyway :) - PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v5]
On Mon, 7 Jun 2021 03:18:32 GMT, Henry Jen wrote: >> …d on macOS >> >> This patch simply round up the specified stack size to multiple of the >> system page size. >> >> Test is trivial, simply run java with -Xss option against following code. On >> MacOS, before the fix, running with `-Xss159k` and `-Xss160k` would get >> `7183` and `649` respectively. After fix, both would output `649`, while >> `-Xss161k` would be same as `-Xss164k` and see 691 as the output. >> >> ```code:java >> public class StackLeak { >> public int depth = 0; >> public void stackLeak() { >> depth++; >> stackLeak(); >> } >> >> public static void main(String[] args) { >> var test = new StackLeak(); >> try { >> test.stackLeak(); >> } catch (Throwable e) { >> System.out.println(test.depth); >> } >> } >> } > > Henry Jen has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains seven additional commits since > the last revision: > > - Cast type > - Merge > - Change java -X output for -Xss > - Merge > - Only try to round-up when current value failed > - Avoid overflow on page size > - JDK-8236569: -Xss not multiple of 4K does not work for the main thread on > macOS Hi Henry, I'm okay with these changes in the current form. The help text needs tweaking - see CSR request. Thanks, David src/java.base/share/classes/sun/launcher/resources/launcher.properties line 175: > 173: \ configuration and continue\n\ > 174: \-Xssset java thread stack size\n\ > 175: \ The actual size may be round up to multiple of > system\n\ See updated help text in the CSR request. - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v5]
On Mon, 7 Jun 2021 03:18:32 GMT, Henry Jen wrote: >> …d on macOS >> >> This patch simply round up the specified stack size to multiple of the >> system page size. >> >> Test is trivial, simply run java with -Xss option against following code. On >> MacOS, before the fix, running with `-Xss159k` and `-Xss160k` would get >> `7183` and `649` respectively. After fix, both would output `649`, while >> `-Xss161k` would be same as `-Xss164k` and see 691 as the output. >> >> ```code:java >> public class StackLeak { >> public int depth = 0; >> public void stackLeak() { >> depth++; >> stackLeak(); >> } >> >> public static void main(String[] args) { >> var test = new StackLeak(); >> try { >> test.stackLeak(); >> } catch (Throwable e) { >> System.out.println(test.depth); >> } >> } >> } > > Henry Jen has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains seven additional commits since > the last revision: > > - Cast type > - Merge > - Change java -X output for -Xss > - Merge > - Only try to round-up when current value failed > - Avoid overflow on page size > - JDK-8236569: -Xss not multiple of 4K does not work for the main thread on > macOS Linux man page for pthread_attr_setstacksize() states that, On some systems, pthread_attr_setstacksize() can fail with the error EINVAL if stacksize is not a multiple of the system page size. - PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v5]
On Mon, 7 Jun 2021 03:22:18 GMT, Henry Jen wrote: > while some other Posix system might as explained in the man page What manpage? The POSIX specification for this does not allow for EINVAL being returned due to alignment issues. That is an extra constraint imposed by macOS and which makes it non-conforming to the POSIX spec IMO. While the changes in src/java.base/unix/native/libjli/java_md.c seem perfectly fine, are we actually ever going to execute it? - PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v5]
On Mon, 7 Jun 2021 03:18:32 GMT, Henry Jen wrote: >> …d on macOS >> >> This patch simply round up the specified stack size to multiple of the >> system page size. >> >> Test is trivial, simply run java with -Xss option against following code. On >> MacOS, before the fix, running with `-Xss159k` and `-Xss160k` would get >> `7183` and `649` respectively. After fix, both would output `649`, while >> `-Xss161k` would be same as `-Xss164k` and see 691 as the output. >> >> ```code:java >> public class StackLeak { >> public int depth = 0; >> public void stackLeak() { >> depth++; >> stackLeak(); >> } >> >> public static void main(String[] args) { >> var test = new StackLeak(); >> try { >> test.stackLeak(); >> } catch (Throwable e) { >> System.out.println(test.depth); >> } >> } >> } > > Henry Jen has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains seven additional commits since > the last revision: > > - Cast type > - Merge > - Change java -X output for -Xss > - Merge > - Only try to round-up when current value failed > - Avoid overflow on page size > - JDK-8236569: -Xss not multiple of 4K does not work for the main thread on > macOS Planned to close JDK-8236569 as 'Won't Fix', as the issue was re-opened, we give it another shot. As explained in the CSR review, we will only round-up the stack size as required by the operating system. Test on Ubuntu shows that there is no need to round-up, while some other Posix system might as explained in the man page. We round-up for MacOS as the document explicitly said that the size need to be multiple of system page size. I also changed to use getpagesize() as you suggested, although that's not needed. - PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v5]
> …d on macOS > > This patch simply round up the specified stack size to multiple of the system > page size. > > Test is trivial, simply run java with -Xss option against following code. On > MacOS, before the fix, running with `-Xss159k` and `-Xss160k` would get > `7183` and `649` respectively. After fix, both would output `649`, while > `-Xss161k` would be same as `-Xss164k` and see 691 as the output. > > ```code:java > public class StackLeak { > public int depth = 0; > public void stackLeak() { > depth++; > stackLeak(); > } > > public static void main(String[] args) { > var test = new StackLeak(); > try { > test.stackLeak(); > } catch (Throwable e) { > System.out.println(test.depth); > } > } > } Henry Jen has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision: - Cast type - Merge - Change java -X output for -Xss - Merge - Only try to round-up when current value failed - Avoid overflow on page size - JDK-8236569: -Xss not multiple of 4K does not work for the main thread on macOS - Changes: - all: https://git.openjdk.java.net/jdk/pull/4256/files - new: https://git.openjdk.java.net/jdk/pull/4256/files/764a1f93..5a8d4a10 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4256&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4256&range=03-04 Stats: 1679 lines in 41 files changed: 1388 ins; 168 del; 123 mod Patch: https://git.openjdk.java.net/jdk/pull/4256.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4256/head:pull/4256 PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v4]
On Sat, 5 Jun 2021 01:48:21 GMT, Henry Jen wrote: >> …d on macOS >> >> This patch simply round up the specified stack size to multiple of the >> system page size. >> >> Test is trivial, simply run java with -Xss option against following code. On >> MacOS, before the fix, running with `-Xss159k` and `-Xss160k` would get >> `7183` and `649` respectively. After fix, both would output `649`, while >> `-Xss161k` would be same as `-Xss164k` and see 691 as the output. >> >> ```code:java >> public class StackLeak { >> public int depth = 0; >> public void stackLeak() { >> depth++; >> stackLeak(); >> } >> >> public static void main(String[] args) { >> var test = new StackLeak(); >> try { >> test.stackLeak(); >> } catch (Throwable e) { >> System.out.println(test.depth); >> } >> } >> } > > Henry Jen has updated the pull request incrementally with one additional > commit since the last revision: > > Change java -X output for -Xss I'm a bit confused about the state of this PR. It was closed/withdrawn but then reopened but with changes applied to platforms other than macOS - why? Also the code does not compile on Linux in current form. - PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v4]
> …d on macOS > > This patch simply round up the specified stack size to multiple of the system > page size. > > Test is trivial, simply run java with -Xss option against following code. On > MacOS, before the fix, running with `-Xss159k` and `-Xss160k` would get > `7183` and `649` respectively. After fix, both would output `649`, while > `-Xss161k` would be same as `-Xss164k` and see 691 as the output. > > ```code:java > public class StackLeak { > public int depth = 0; > public void stackLeak() { > depth++; > stackLeak(); > } > > public static void main(String[] args) { > var test = new StackLeak(); > try { > test.stackLeak(); > } catch (Throwable e) { > System.out.println(test.depth); > } > } > } Henry Jen has updated the pull request incrementally with one additional commit since the last revision: Change java -X output for -Xss - Changes: - all: https://git.openjdk.java.net/jdk/pull/4256/files - new: https://git.openjdk.java.net/jdk/pull/4256/files/1e96be94..764a1f93 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4256&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4256&range=02-03 Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4256.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4256/head:pull/4256 PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v3]
> …d on macOS > > This patch simply round up the specified stack size to multiple of the system > page size. > > Test is trivial, simply run java with -Xss option against following code. On > MacOS, before the fix, running with `-Xss159k` and `-Xss160k` would get > `7183` and `649` respectively. After fix, both would output `649`, while > `-Xss161k` would be same as `-Xss164k` and see 691 as the output. > > ```code:java > public class StackLeak { > public int depth = 0; > public void stackLeak() { > depth++; > stackLeak(); > } > > public static void main(String[] args) { > var test = new StackLeak(); > try { > test.stackLeak(); > } catch (Throwable e) { > System.out.println(test.depth); > } > } > } Henry Jen has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision: - Merge - Only try to round-up when current value failed - Avoid overflow on page size - JDK-8236569: -Xss not multiple of 4K does not work for the main thread on macOS - Changes: - all: https://git.openjdk.java.net/jdk/pull/4256/files - new: https://git.openjdk.java.net/jdk/pull/4256/files/39b041d7..1e96be94 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4256&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4256&range=01-02 Stats: 485003 lines in 2409 files changed: 450413 ins; 28438 del; 6152 mod Patch: https://git.openjdk.java.net/jdk/pull/4256.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4256/head:pull/4256 PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS [v2]
> …d on macOS > > This patch simply round up the specified stack size to multiple of the system > page size. > > Test is trivial, simply run java with -Xss option against following code. On > MacOS, before the fix, running with `-Xss159k` and `-Xss160k` would get > `7183` and `649` respectively. After fix, both would output `649`, while > `-Xss161k` would be same as `-Xss164k` and see 691 as the output. > > ```code:java > public class StackLeak { > public int depth = 0; > public void stackLeak() { > depth++; > stackLeak(); > } > > public static void main(String[] args) { > var test = new StackLeak(); > try { > test.stackLeak(); > } catch (Throwable e) { > System.out.println(test.depth); > } > } > } Henry Jen has updated the pull request incrementally with one additional commit since the last revision: Avoid overflow on page size - Changes: - all: https://git.openjdk.java.net/jdk/pull/4256/files - new: https://git.openjdk.java.net/jdk/pull/4256/files/fb9b22d5..39b041d7 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4256&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4256&range=00-01 Stats: 15 lines in 2 files changed: 8 ins; 0 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/4256.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4256/head:pull/4256 PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS
On Fri, 28 May 2021 21:55:24 GMT, Henry Jen wrote: > …d on macOS > > This patch simply round up the specified stack size to multiple of the system > page size. > > Test is trivial, simply run java with -Xss option against following code. On > MacOS, before the fix, running with `-Xss159k` and `-Xss160k` would get > `7183` and `649` respectively. After fix, both would output `649`, while > `-Xss161k` would be same as `-Xss164k` and see 691 as the output. > > ```code:java > public class StackLeak { > public int depth = 0; > public void stackLeak() { > depth++; > stackLeak(); > } > > public static void main(String[] args) { > var test = new StackLeak(); > try { > test.stackLeak(); > } catch (Throwable e) { > System.out.println(test.depth); > } > } > } Withdraw for keep current behavior for compatibility. It would be preferred for user to specify proper value than we change the value on user's behalf. - PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS
On Mon, 31 May 2021 20:23:53 GMT, Vladimir Kempik wrote: >> src/java.base/macosx/native/libjli/java_md_macosx.m line 727: >> >>> 725: >>> 726: static size_t alignUp(size_t stack_size) { >>> 727: long page_size = sysconf(_SC_PAGESIZE); >> >> In hotspot we use `getpagesize()`. There is also a guard for a very large >> stack (within a page of SIZE_MAX) so that rounding up does not produce zero. > > sounds like that (getpagesize) should work with m1 mac as well, as they have > 16k pages. will it ? sysconf is the portable way based on POSIX, we can use getpagesize give this is macOS specific code, which is BSD based. - PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS
On Sun, 30 May 2021 03:00:56 GMT, David Holmes wrote: >> …d on macOS >> >> This patch simply round up the specified stack size to multiple of the >> system page size. >> >> Test is trivial, simply run java with -Xss option against following code. On >> MacOS, before the fix, running with `-Xss159k` and `-Xss160k` would get >> `7183` and `649` respectively. After fix, both would output `649`, while >> `-Xss161k` would be same as `-Xss164k` and see 691 as the output. >> >> ```code:java >> public class StackLeak { >> public int depth = 0; >> public void stackLeak() { >> depth++; >> stackLeak(); >> } >> >> public static void main(String[] args) { >> var test = new StackLeak(); >> try { >> test.stackLeak(); >> } catch (Throwable e) { >> System.out.println(test.depth); >> } >> } >> } > > src/java.base/macosx/native/libjli/java_md_macosx.m line 727: > >> 725: >> 726: static size_t alignUp(size_t stack_size) { >> 727: long page_size = sysconf(_SC_PAGESIZE); > > In hotspot we use `getpagesize()`. There is also a guard for a very large > stack (within a page of SIZE_MAX) so that rounding up does not produce zero. sounds like that (getpagesize) should work with m1 mac as well, as they have 16k pages. will it ? - PR: https://git.openjdk.java.net/jdk/pull/4256
Re: RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS
On Fri, 28 May 2021 21:55:24 GMT, Henry Jen wrote: > …d on macOS > > This patch simply round up the specified stack size to multiple of the system > page size. > > Test is trivial, simply run java with -Xss option against following code. On > MacOS, before the fix, running with `-Xss159k` and `-Xss160k` would get > `7183` and `649` respectively. After fix, both would output `649`, while > `-Xss161k` would be same as `-Xss164k` and see 691 as the output. > > ```code:java > public class StackLeak { > public int depth = 0; > public void stackLeak() { > depth++; > stackLeak(); > } > > public static void main(String[] args) { > var test = new StackLeak(); > try { > test.stackLeak(); > } catch (Throwable e) { > System.out.println(test.depth); > } > } > } src/java.base/macosx/native/libjli/java_md_macosx.m line 727: > 725: > 726: static size_t alignUp(size_t stack_size) { > 727: long page_size = sysconf(_SC_PAGESIZE); In hotspot we use `getpagesize()`. There is also a guard for a very large stack (within a page of SIZE_MAX) so that rounding up does not produce zero. - PR: https://git.openjdk.java.net/jdk/pull/4256
RFR: 8236569: -Xss not multiple of 4K does not work for the main thread on macOS
…d on macOS This patch simply round up the specified stack size to multiple of the system page size. Test is trivial, simply run java with -Xss option against following code. On MacOS, before the fix, running with `-Xss159k` and `-Xss160k` would get `7183` and `649` respectively. After fix, both would output `649`, while `-Xss161k` would be same as `-Xss164k` and see 691 as the output. ```code:java public class StackLeak { public int depth = 0; public void stackLeak() { depth++; stackLeak(); } public static void main(String[] args) { var test = new StackLeak(); try { test.stackLeak(); } catch (Throwable e) { System.out.println(test.depth); } } } - Commit messages: - JDK-8236569: -Xss not multiple of 4K does not work for the main thread on macOS Changes: https://git.openjdk.java.net/jdk/pull/4256/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4256&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8236569 Stats: 23 lines in 2 files changed: 21 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/4256.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4256/head:pull/4256 PR: https://git.openjdk.java.net/jdk/pull/4256