On Sun, 9 Aug 2026 10:16:50 GMT, Yasumasa Suenaga <[email protected]> wrote:

> We've discussed about them in #32210, and we've reached the conclusion to add 
> `--enable-preview` into the argument of 
> `createLimitedTestJavaProcessBuilder`. LingeredApp should be kicked as an 
> another process because the test have to attach it to test SA features. It is 
> out of control from jtreg. Thus we need to add `--enable-preview` if needs.
> 
> As you know SA is like a debugger, so command line arguments of debuggee 
> (LingeredApp) might be different from jtreg test.

Thanks for providing the additional context, I was not aware of this 
discussion. 

As far as I understand, the flow is as follows:
- The test driver (`TestInstanceKlassSize`) is launched as `main/othervm`, 
taking a few arguments + whatever `JTREG=` has.
- The driver spawns two subprocesses:
  - The `LingeredApp`, with the same arguments as the driver (i.e., `JTREG=` is 
included).
  - Itself again, but this time with the intent to do the actual testing logic 
rather than launching, _without_ the `JTREG=` arguments. 

In that case, I see why checking for preview manually is the easiest way 
forward to address these symptoms. My main concern lies with the 
maintainability, as this is a common pattern spinning up tests. I know from 
experience that bolting on code to make it work for Valhalla can quickly become 
ugly/inconsistent. Taking 
`test/hotspot/jtreg/serviceability/sa/TestG1HeapRegion.java` as an example, 
shouldn't this also propagate `--enable-preview`? Did we just get lucky that 
this test passes in the "wrong" configuration? Would it make sense to introduce 
some sort of utility to the SA tests that wraps 
`createLimitedTestJavaProcessBuilder` and automatically does this, rather than 
having to do it at the call site of `createLimitedTestJavaProcessBuilder` in 
each test? How would you feel about a follow-up RFE (can assign to me) for this?

On a side note, these tests look like the usual candidate that we normally 
`@require vm.flagless`. I assume we omitted this because we want the 
possibility to run `LingeredApp` with nonstandard JVM arguments? Or was this an 
oversight?

-------------

PR Comment: https://git.openjdk.org/jdk/pull/32262#issuecomment-5231761842

Reply via email to