Re: RFR: 8331087: Move immutable nmethod data from CodeCache [v2]

2024-04-28 Thread Vladimir Kozlov
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]

2024-04-28 Thread Vladimir Kozlov
> 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=18984=01
 - incr: https://webrevs.openjdk.org/?repo=jdk=18984=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


Re: RFR: 8226919: attach in linux hangs due to permission denied accessing /proc/pid/root

2024-04-28 Thread Sebastian Lövdahl
On Fri, 1 Mar 2024 15:22:51 GMT, jdoylei  wrote:

>> Logged https://bugs.openjdk.org/browse/JDK-8327114 for investigation.  
>> Thanks @jdoylei  !
>
> @kevinjwalls - Perfect, thank you for opening the JBS bug!

Thanks for the detailed write-up, @jdoylei! I'm sorry to have introduced a 
regression here. Good that the backporting was held off a bit at least :) Let's 
continue the discussion at 
https://mail.openjdk.org/pipermail/serviceability-dev/2024-April/055317.html.

-

PR Comment: https://git.openjdk.org/jdk/pull/17628#issuecomment-2081618227


8327114: Attach in Linux may have wrong behaviour when pid == ns_pid (Kubernetes debug container)

2024-04-28 Thread Sebastian Lövdahl

Hi all,

It seems like my fix for https://bugs.openjdk.org/browse/JDK-8226919 
regressed one use-case for Kubernetes debug containers (and other 
technically similar approaches). Quoting @jdoylei from 
https://github.com/openjdk/jdk/pull/17628#issuecomment-1969769654:


"We're running jcmd (OpenJDK build 17.0.10+7-LTS) and the target JVM in 
two separate containers in a Kubernetes pod. The target JVM container is 
already running, and then we use kubectl debug --target=... to start a 
Kubernetes debug container with jcmd that targets the first container. 
Given the --target option, they share the same Linux process namespace 
(both think the target JVM is PID 1). But since they are separate 
containers, they see different root filesystems (jcmd container sees the 
target JVM tmpdir under /proc/1/root/tmp but has its own distinct /tmp 
directory)."


I think I can confirm that there is a regression. Using a locally built 
JDK from master as of 2024-04-28 
(16c7dcdb04a7c220684a20eb4a0da4505ae03813), but using raw Docker 
containers instead of Kubernetes + kubectl debug:



slovdahl@ubuntu2204:~/reproducer$ cat Reproducer.java
import java.io.IOException;
import java.net.InetSocketAddress;
import java.net.ServerSocket;

public class Reproducer {
  public static void main(String[] args) throws InterruptedException, 
IOException {

    System.out.println("Hello, World!");
    try (var server = new ServerSocket()) {
  server.bind(new InetSocketAddress("localhost", 81));
  System.out.println("Bound to port 81");
  while (true) {
    Thread.sleep(1_000L);
  }
    }
  }
}

slovdahl@ubuntu2204:~/reproducer$ docker run --interactive --tty --rm 
--name app-container --volume 
~/jdk/build/linux-x86_64-server-release/images/jdk/:/jdk --volume .:/app 
--workdir /app ubuntu:22.04 /bin/bash

root@d1f87b8059ea:/app# /jdk/bin/java -version
openjdk version "23-internal" 2024-09-17
OpenJDK Runtime Environment (build 23-internal-adhoc.slovdahl.jdk)
OpenJDK 64-Bit Server VM (build 23-internal-adhoc.slovdahl.jdk, mixed 
mode, sharing)


root@d1f87b8059ea:/app# /jdk/bin/java Reproducer.java
Hello, World!
Bound to port 81


Locally built JDK and jcmd from the host (works):

slovdahl@ubuntu2204:~/reproducer$ sudo 
~/jdk/build/linux-x86_64-server-release/images/jdk/bin/jcmd 942781 
VM.version

942781:
OpenJDK 64-Bit Server VM version 23-internal-adhoc.slovdahl.jdk
JDK 23.0.0

jcmd from a sidecar Docker container mounted into the same process 
namespace (does NOT work, regressed):


slovdahl@ubuntu2204:~/reproducer$ docker run --interactive --tty --rm 
--pid=container:app-container --volume 
~/jdk/build/linux-x86_64-server-release/images/jdk/:/jdk ubuntu:22.04 
/bin/bash

root@27d8be9186b7:/# /jdk/bin/jcmd
26 jdk.compiler/com.sun.tools.javac.launcher.SourceLauncher Reproducer.java
59 jdk.jcmd/sun.tools.jcmd.JCmd
root@27d8be9186b7:/# /jdk/bin/jcmd 26 VM.version
26:
com.sun.tools.attach.AttachNotSupportedException: Unable to open socket 
file /tmp/.java_pid26: target process 26 doesn't respond within 10500ms 
or HotSpot VM not loaded
    at 
jdk.attach/sun.tools.attach.VirtualMachineImpl.(VirtualMachineImpl.java:99)
    at 
jdk.attach/sun.tools.attach.AttachProviderImpl.attachVirtualMachine(AttachProviderImpl.java:58)
    at 
jdk.attach/com.sun.tools.attach.VirtualMachine.attach(VirtualMachine.java:207)

    at jdk.jcmd/sun.tools.jcmd.JCmd.executeCommandForPid(JCmd.java:113)
    at jdk.jcmd/sun.tools.jcmd.JCmd.main(JCmd.java:97)


Using Temurin 17.0.11 from the host (works):

slovdahl@ubuntu2204:~/reproducer$ 
/usr/lib/jvm/temurin-17-jdk-amd64/bin/java -version

openjdk version "17.0.11" 2024-04-16
OpenJDK Runtime Environment Temurin-17.0.11+9 (build 17.0.11+9)
OpenJDK 64-Bit Server VM Temurin-17.0.11+9 (build 17.0.11+9, mixed mode, 
sharing)
slovdahl@ubuntu2204:~/reproducer$ sudo 
/usr/lib/jvm/temurin-17-jdk-amd64/bin/jcmd 942781 VM.version

942781:
OpenJDK 64-Bit Server VM version 23-internal-adhoc.slovdahl.jdk
JDK 23.0.0


Temurin 17.0.11 jcmd from a sidecar Docker container mounted into the 
same process namespace (works):


slovdahl@ubuntu2204:~/reproducer$ docker run --interactive --tty --rm 
--pid=container:app-container eclipse-temurin:17.0.11_9-jdk-jammy /bin/bash

root@fcbd6e4be9eb:/# java -version
openjdk version "17.0.11" 2024-04-16
OpenJDK Runtime Environment Temurin-17.0.11+9 (build 17.0.11+9)
OpenJDK 64-Bit Server VM Temurin-17.0.11+9 (build 17.0.11+9, mixed mode, 
sharing)

root@fcbd6e4be9eb:/# jcmd
138 jdk.jcmd/sun.tools.jcmd.JCmd
26 jdk.compiler/com.sun.tools.javac.launcher.SourceLauncher Reproducer.java
root@fcbd6e4be9eb:/# jcmd 26 VM.version
26:
OpenJDK 64-Bit Server VM version 23-internal-adhoc.slovdahl.jdk
JDK 23.0.0


Curiously enough, there is a test that on the surface seemed to be 
written specifically for this case 
(test/hotspot/jtreg/containers/docker/TestJcmdWithSideCar.java). But the 
devil is in the details: in TestJcmdWithSideCar /tmp in the main 
container is a volume that is 

Re: RFR: 8331087: Move immutable nmethod data from CodeCache

2024-04-28 Thread Doug Simon
On Sun, 28 Apr 2024 07:02:40 GMT, Dean Long  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.
>
> It only makes sense if the immutable data heap is not also used for other 
> critical resources.  If malloc or metaspace were used as the immutable data 
> heap, normally failures in those heaps are fatal, because other critical 
> resources (monitors, classes, etc) are allocated from there, so any failure 
> means the JVM is about to die.  There's no reason to find a fall-back method 
> to allocate a new nmethod in that case.

Just to be clear @dean-long , you're saying failure to allocate immutable data 
in the C heap should result in a fatal error? Makes sense to me as the VM must 
indeed be very close to crashing anyway in that case. It also, obviates the 
need for propagating `out_of_memory_error` to JVMCI code.

-

PR Comment: https://git.openjdk.org/jdk/pull/18984#issuecomment-2081427477


Re: RFR: 8331087: Move immutable nmethod data from CodeCache

2024-04-28 Thread Dean Long
On Fri, 26 Apr 2024 21:16:03 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.

It only makes sense if the immutable data heap is not also used for other 
critical resources.  If malloc or metaspace were used as the immutable data 
heap, normally failures in those heaps are fatal, because other critical 
resources (monitors, classes, etc) are allocated from there, so any failure 
means the JVM is about to die.  There's no reason to find a fall-back method to 
allocate a new nmethod in that case.

-

PR Comment: https://git.openjdk.org/jdk/pull/18984#issuecomment-2081364009