Update !vthread command to use GC continuation list

[thallium]

Fixes #17084

[keithc-ca]

I created the test program below to see how well this works. A significant issue is that only virtual threads created before a GC cycle are displayed, for example running

java --enable-preview TestVirtual 4 6

yields a core file that claims only 4 virtual threads exist.

import java.util.ArrayList;
import java.util.List;

public class TestVirtual {

	private static final class Helper {

		private static void sayHi(int id) {
			sleep(id * 10 + 250);
			System.out.format("Virtual thread #%d says 'Hi'.%n", id);
			sleep(500);
		}

		private final Thread.Builder builder;

		private final List<Thread> threads;

		@SuppressWarnings("preview")
		Helper() {
			super();
			builder = Thread.ofVirtual();
			threads = new ArrayList<>();
		}

		void addThreads(int count) {
			int base = threads.size();

			for (int i = 0; i < count; ++i) {
				int id = base + i;
				threads.add(builder.name("greeter#" + id).unstarted(() -> sayHi(id)));
			}
		}

		void joinThreads() {
			for (Thread thread : threads) {
				try {
					thread.join();
				} catch (InterruptedException e) {
					// ignore
				}
			}
		}

		void startThreads() {
			for (Thread thread : threads) {
				thread.start();
			}
		}

	}

	public static void main(String[] args) {
		int beforeGC = (args.length > 0) ? Integer.parseInt(args[0]) : 10;
		int afterGC = (args.length > 1) ? Integer.parseInt(args[1]) : 0;

		if ((beforeGC < 0) || (afterGC < 0)) {
			System.out.println("Thread counts must be non-negative.");
			return;
		} else if ((beforeGC == 0) && (afterGC == 0)) {
			System.out.println("Total thread count must be positive.");
			return;
		}

		Helper helper = new Helper();

		System.out.println("Creating threads...");

		if (beforeGC > 0) {
			helper.addThreads(beforeGC);

			sleep(100);

			System.out.println("Collecting garbage...");
			System.gc();
		}

		if (afterGC > 0) {
			if (beforeGC > 0) {
				System.out.println("Creating more threads...");
			}

			helper.addThreads(afterGC);
			sleep(100);
		}

		System.out.println("Starting threads...");
		helper.startThreads();

		com.ibm.jvm.Dump.systemDumpToFile();

		System.out.println("Joining threads...");
		helper.joinThreads();

		System.out.println("Done.");
	}

	private static void sleep(long ms) {
		try {
			Thread.sleep(ms);
		} catch (InterruptedException e) {
			// ignore
		}
	}

}

[babsingh]

The virtual threads after the GC don't show up because most likely the nonAllocationCaches are not flushed before core generation.

Ref: #17084 (comment)

I believe nonAllocationCaches should be flushed already before core generation. There is very similar logic for Ownable Synchronizers lists in gccheck can be used as example.

There may be code-paths where the nonAllocationCaches are not flushed before core generation. @dmitripivkine Can you please point us to existing locations where the nonAllocationCaches are flushed before core generation?

[dmitripivkine]

There may be code-paths where the nonAllocationCaches are not flushed before core generation. @dmitripivkine Can you please point us to existing locations where the nonAllocationCaches are flushed before core generation?

There is the place I have in mind:

openj9/runtime/rasdump/trigger.c

Line 683 in 4075b02

vm->memoryManagerFunctions->j9gc_flush_caches_for_walk(vm);

So it is required prepwalk to be enabled, for gpf for instance.

And it is not j9gc_flush_nonAllocationCaches_for_walk(), my mistake.
So, I guess we need to find place to call it then.

[thallium]

So, I guess we need to find place to call it then.

@dmitripivkine Is some form of synchronization needed to call j9gc_flush_nonAllocationCaches_for_walk()?

[dmitripivkine]

So, I guess we need to find place to call it then.

@dmitripivkine Is some form of synchronization needed to call j9gc_flush_nonAllocationCaches_for_walk()?

Yes, it is. It should be safe to call if J9RAS_DUMP_GOT_EXCLUSIVE_VM_ACCESS flag is set. However there are number if reasons for core generation (GPF, on-sygnal, trace-assert) where there is no attempt to get Exclusive due the possible hang. It means these cases would not be covered.

Please note that the way how Virtual Threads list (currently in GC) is organized is going to be re-worked very soon. It means current implementation is going to be replaced to different one in nearest feature which requires new DDR code any way.

So, if we need short term temporary solution, my suggestion is to make improvement by calling j9gc_flush_nonAllocationCaches_for_walk() when J9RAS_DUMP_GOT_EXCLUSIVE_VM_ACCESS is set (similar to j9gc_flush_caches_for_walk() call) and stop here even some cases are not going to be covered. There is no reason to design new code in DDR (like manual flush of Sublists for each thread) which will be unnecessary very soon. At the time when new Virtual Threads list implementation is going to be in place we should revisit this part of DDR and provide a complete coverage.

[keithc-ca]

Please note that the way how Virtual Threads list (currently in GC) is organized is going to be re-worked very soon.

Is there an issue related to that upcoming work?

[dmitripivkine]

Please note that the way how Virtual Threads list (currently in GC) is organized is going to be re-worked very soon.

Is there an issue related to that upcoming work?

I don't think we have formalized issue opened yet

[babsingh]

With the new change and test from #17093 (comment), are all virtual threads seen in the core file?

[thallium]

With the new change and test from #17093 (comment), are all virtual threads seen in the core file?

Unfortunately we're still only seeing four threads.

[keithc-ca]

With the new change and test from #17093 (comment), are all virtual threads seen in the core file?

Yes, but only if the test is changed to call

  com.ibm.jvm.Dump.setDumpOptions("system:request=exclusive+prepwalk");

before triggering the system dump or the test is run with -Xdump:system:request=exclusive+prepwalk.

Perhaps that is sufficient for now.

@dmitripivkine Please create that issue so we can discuss those plans and ensure that this limitation is removed.

[keithc-ca]

Jenkins test sanity alinux64 jdk19

[pshipton]

Pls cherry pick this change and open a PR against https://github.com/eclipse-openj9/openj9/tree/v0.37.0-release

[thallium]

Pls cherry pick this change and open a PR against https://github.com/eclipse-openj9/openj9/tree/v0.37.0-release

This DDR command was not initially included in that branch. Do we still want to back-port?

[pshipton]

Do we still want to back-port?

What's involved, what other PR is needed? If it's just DDR code there is no reason not to backport.

[thallium]

#15679 is the original PR, it's just DDR code and a helper.

[dmitripivkine]

@dmitripivkine Please create that issue so we can discuss those plans and ensure that this limitation is removed.

#17287