Re: RFR: JDK-8324637: [aix] Implement support for reporting swap space in jdk.management
On Thu, 25 Jan 2024 15:33:36 GMT, Matthias Baesken wrote: > > I can't try this and don't use AIX, but it looks good. It follows the same > > pattern as the other AIX cases in the file. > > Although the others (e.g. line 200) don't throw_internal_error if the call > > returns -1, they just return -1. You might want to do that for consistency > > of behaviour. > > Hi I wanted to be consistent to the other OS in > get_total_or_available_swap_space_size, that#s why the throw_internal_error . Ok yes, you can choose who to be consistent with... 8-) - PR Comment: https://git.openjdk.org/jdk/pull/17569#issuecomment-1910585693
Re: RFR: JDK-8324637: [aix] Implement support for reporting swap space in jdk.management
On Thu, 25 Jan 2024 14:25:51 GMT, Thomas Stuefe wrote: > Do we need the cast? perfstat_memory_total_t members are all 64-bit, no? > > Also, can we shorten this to: > > ``` > return (available ? memory_info.pgsp_free : memory_info.pgsp_total) * 4096; > ``` Hi Thomas, I see no types defined here https://www.ibm.com/docs/pt/aix/7.2?topic=interfaces-perfstat-memory-total-interface but most likely you are correct and they are 64bit. - PR Review Comment: https://git.openjdk.org/jdk/pull/17569#discussion_r1466552393
Re: RFR: JDK-8324637: [aix] Implement support for reporting swap space in jdk.management
On Thu, 25 Jan 2024 14:11:35 GMT, Kevin Walls wrote: > I can't try this and don't use AIX, but it looks good. It follows the same > pattern as the other AIX cases in the file. > > Although the others (e.g. line 200) don't throw_internal_error if the call > returns -1, they just return -1. You might want to do that for consistency of > behaviour. Hi I wanted to be consistent to the other OS in get_total_or_available_swap_space_size, that#s why the throw_internal_error . - PR Comment: https://git.openjdk.org/jdk/pull/17569#issuecomment-1910449037
Re: RFR: JDK-8324637: [aix] Implement support for reporting swap space in jdk.management
On Thu, 25 Jan 2024 12:30:15 GMT, Matthias Baesken wrote: > The get_total_or_available_swap_space_size coding misses AIX support, we only > return 0. This should be enhanced. > The perfstat API can be used, see > https://www.ibm.com/docs/pt/aix/7.2?topic=interfaces-perfstat-memory-total-interface > . Small nit, otherwise good. src/jdk.management/unix/native/libmanagement_ext/OperatingSystemImpl.c line 113: > 111: throw_internal_error(env, "perfstat_memory_total failed"); > 112: } > 113: return available ? (jlong)(memory_info.pgsp_free * 4L * 1024L) : > (jlong)(memory_info.pgsp_total * 4L * 1024L); Do we need the cast? perfstat_memory_total_t members are all 64-bit, no? Also, can we shorten this to: return (available ? memory_info.pgsp_free : memory_info.pgsp_total) * 4096; - Marked as reviewed by stuefe (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17569#pullrequestreview-1843868729 PR Review Comment: https://git.openjdk.org/jdk/pull/17569#discussion_r1466454083
Re: RFR: JDK-8324637: [aix] Implement support for reporting swap space in jdk.management
On Thu, 25 Jan 2024 12:30:15 GMT, Matthias Baesken wrote: > The get_total_or_available_swap_space_size coding misses AIX support, we only > return 0. This should be enhanced. > The perfstat API can be used, see > https://www.ibm.com/docs/pt/aix/7.2?topic=interfaces-perfstat-memory-total-interface > . I can't try this and don't use AIX, but it looks good. It follows the same pattern as the other AIX cases in the file. Although the others (e.g. line 200) don't throw_internal_error if the call returns -1, they just return -1. You might want to do that for consistency of behaviour. - Marked as reviewed by kevinw (Committer). PR Review: https://git.openjdk.org/jdk/pull/17569#pullrequestreview-1843836792
