Re: RFR: 8331087: Move immutable nmethod data from CodeCache [v2]
On Sun, 28 Apr 2024 23:37:22 GMT, Vladimir Kozlov wrote: >> Move immutable nmethod's data from CodeCache to C heap. It includes >> `dependencies, nul_chk_table, handler_table, scopes_pcs, scopes_data, >> speculations`. It amounts for about 30% (optimized VM) of space in CodeCache. >> >> Use HotSpot's `os::malloc()` to allocate memory in C heap for immutable >> nmethod's data. Call `vm_exit_out_of_memory()` if allocation failed. >> >> Shuffle fields order and change some fields size from 4 to 2 bytes to avoid >> nmethod's header size increase. >> >> Tested tier1-5, stress,xcomp >> >> Our performance testing does not show difference. >> >> Example of updated `-XX:+PrintNMethodStatistics` output is in JBS comment. > > Vladimir Kozlov has updated the pull request incrementally with one > additional commit since the last revision: > > Address comments. Moved jvmci_data back to mutable data section. Thank you, Dean, Doug and Tobias for reviews. - PR Comment: https://git.openjdk.org/jdk/pull/18984#issuecomment-2083102730
Re: RFR: 8331087: Move immutable nmethod data from CodeCache [v2]
On Mon, 29 Apr 2024 06:33:09 GMT, Tobias Hartmann wrote: > Looks good to me. Did you measure any impact on performance (potentially due > to improved code density)? Thank you, @TobiHartmann, for review. > What's left for [JDK-7072317](https://bugs.openjdk.org/browse/JDK-7072317)? Make Relocation info (10% of nmethod size) immutable by moving all encoded pointers (external words and others, which we need to patch in Leyden when loading cached code) from it into separate mutable section. > I wonder if the `CHECKED_CAST` changes shouldn't go into a separate RFE. No, I want clear information which cast failed instead of trying to reproduce very intermittent failure like this: [JDK-8331253](https://bugs.openjdk.org/browse/JDK-8331253). When you have several `checked_cast` in one method it is impossible to find which failed without this macro. - PR Comment: https://git.openjdk.org/jdk/pull/18984#issuecomment-2082953366
Re: RFR: 8331087: Move immutable nmethod data from CodeCache [v2]
On Sun, 28 Apr 2024 23:37:22 GMT, Vladimir Kozlov wrote: >> Move immutable nmethod's data from CodeCache to C heap. It includes >> `dependencies, nul_chk_table, handler_table, scopes_pcs, scopes_data, >> speculations`. It amounts for about 30% (optimized VM) of space in CodeCache. >> >> Use HotSpot's `os::malloc()` to allocate memory in C heap for immutable >> nmethod's data. Call `vm_exit_out_of_memory()` if allocation failed. >> >> Shuffle fields order and change some fields size from 4 to 2 bytes to avoid >> nmethod's header size increase. >> >> Tested tier1-5, stress,xcomp >> >> Our performance testing does not show difference. >> >> Example of updated `-XX:+PrintNMethodStatistics` output is in JBS comment. > > Vladimir Kozlov has updated the pull request incrementally with one > additional commit since the last revision: > > Address comments. Moved jvmci_data back to mutable data section. Marked as reviewed by dnsimon (Reviewer). JVMCI changes now look good to me. - PR Review: https://git.openjdk.org/jdk/pull/18984#pullrequestreview-2027884309 PR Comment: https://git.openjdk.org/jdk/pull/18984#issuecomment-2082185011
Re: RFR: 8331087: Move immutable nmethod data from CodeCache [v2]
On Sun, 28 Apr 2024 23:37:22 GMT, Vladimir Kozlov wrote: >> Move immutable nmethod's data from CodeCache to C heap. It includes >> `dependencies, nul_chk_table, handler_table, scopes_pcs, scopes_data, >> speculations`. It amounts for about 30% (optimized VM) of space in CodeCache. >> >> Use HotSpot's `os::malloc()` to allocate memory in C heap for immutable >> nmethod's data. Call `vm_exit_out_of_memory()` if allocation failed. >> >> Shuffle fields order and change some fields size from 4 to 2 bytes to avoid >> nmethod's header size increase. >> >> Tested tier1-5, stress,xcomp >> >> Our performance testing does not show difference. >> >> Example of updated `-XX:+PrintNMethodStatistics` output is in JBS comment. > > Vladimir Kozlov has updated the pull request incrementally with one > additional commit since the last revision: > > Address comments. Moved jvmci_data back to mutable data section. Marked as reviewed by dlong (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18984#pullrequestreview-2027786061
Re: RFR: 8331087: Move immutable nmethod data from CodeCache [v2]
On Sun, 28 Apr 2024 23:37:22 GMT, Vladimir Kozlov wrote: >> Move immutable nmethod's data from CodeCache to C heap. It includes >> `dependencies, nul_chk_table, handler_table, scopes_pcs, scopes_data, >> speculations`. It amounts for about 30% (optimized VM) of space in CodeCache. >> >> Use HotSpot's `os::malloc()` to allocate memory in C heap for immutable >> nmethod's data. Call `vm_exit_out_of_memory()` if allocation failed. >> >> Shuffle fields order and change some fields size from 4 to 2 bytes to avoid >> nmethod's header size increase. >> >> Tested tier1-5, stress,xcomp >> >> Our performance testing does not show difference. >> >> Example of updated `-XX:+PrintNMethodStatistics` output is in JBS comment. > > Vladimir Kozlov has updated the pull request incrementally with one > additional commit since the last revision: > > Address comments. Moved jvmci_data back to mutable data section. Looks good to me. Did you measure any impact on performance (potentially due to improved code density)? What's left for [JDK-7072317](https://bugs.openjdk.org/browse/JDK-7072317)? I wonder if the `CHECKED_CAST` changes shouldn't go into a separate RFE. - Marked as reviewed by thartmann (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18984#pullrequestreview-2027660174
Re: RFR: 8331087: Move immutable nmethod data from CodeCache [v2]
On Sun, 28 Apr 2024 23:37:22 GMT, Vladimir Kozlov wrote: >> Move immutable nmethod's data from CodeCache to C heap. It includes >> `dependencies, nul_chk_table, handler_table, scopes_pcs, scopes_data, >> speculations, jvmci_data`. It amounts for about 30% (optimized VM) of space >> in CodeCache. >> >> Use HotSpot's `os::malloc()` to allocate memory in C heap for immutable >> nmethod's data. Bail out compilation if allocation failed. >> >> Shuffle fields order and change some fields size from 4 to 2 bytes to avoid >> nmethod's header size increase. >> >> Tested tier1-5, stress,xcomp >> >> Our performance testing does not show difference. >> >> Example of updated `-XX:+PrintNMethodStatistics` output is in JBS comment. > > Vladimir Kozlov has updated the pull request incrementally with one > additional commit since the last revision: > > Address comments. Moved jvmci_data back to mutable data section. Update: 1. Addressed @dean-long first comments. 2. Based on discussion with Doug and Tom (see comments in [JDK-8331087](https://bugs.openjdk.org/browse/JDK-8331087)), moved `jvmci_data` back to nmethod's mutable data section. 3. Replaced my allocation failure handling code with call to `vm_exit_out_of_memory()`. I verified (with `UseNewCode` hack`) that out of memory is correctly reported in fastdebug and product VMs: # # There is insufficient memory for the Java Runtime Environment to continue. # Native memory allocation (malloc) failed to allocate 64 bytes. Error detail: nmethod: no space for immutable data # An error report file with more information is saved as: # /scratch/kvn/jdk_git/hs_err_pid4086275.log - PR Comment: https://git.openjdk.org/jdk/pull/18984#issuecomment-2081701059
Re: RFR: 8331087: Move immutable nmethod data from CodeCache [v2]
> Move immutable nmethod's data from CodeCache to C heap. It includes > `dependencies, nul_chk_table, handler_table, scopes_pcs, scopes_data, > speculations, jvmci_data`. It amounts for about 30% (optimized VM) of space > in CodeCache. > > Use HotSpot's `os::malloc()` to allocate memory in C heap for immutable > nmethod's data. Bail out compilation if allocation failed. > > Shuffle fields order and change some fields size from 4 to 2 bytes to avoid > nmethod's header size increase. > > Tested tier1-5, stress,xcomp > > Our performance testing does not show difference. > > Example of updated `-XX:+PrintNMethodStatistics` output is in JBS comment. Vladimir Kozlov has updated the pull request incrementally with one additional commit since the last revision: Address comments. Moved jvmci_data back to mutable data section. - Changes: - all: https://git.openjdk.org/jdk/pull/18984/files - new: https://git.openjdk.org/jdk/pull/18984/files/6b1f69d9..1824f46c Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=18984&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18984&range=00-01 Stats: 98 lines in 5 files changed: 39 ins; 36 del; 23 mod Patch: https://git.openjdk.org/jdk/pull/18984.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18984/head:pull/18984 PR: https://git.openjdk.org/jdk/pull/18984
