RFR: 8320750: Allow a testcase to run with muliple -Xlog

[Leo Korinth]

On Mon, 27 Nov 2023 13:32:52 GMT, Leo Korinth <lkorinth at openjdk.org> wrote:

> Running a testcase with muliple -Xlog crashes JTREG test cases. This is because `Collector.toMap` is not given a merge strategy.
> 
> When the same argument is passed multiple times, I have added a merge strategy to use the latter value. This is similar to how it is implemented for `vm.opt.*` in JTREG.
> 
> If the flag tested is `-Xlog`, replace the value part with a dummy value "NONEMPTY_TEST_SENTINEL". This is because in the case of multiple `-Xlog` all values are used, and JTREG does not give a satisfactory way to represent them. This dummy value should make it hard to try to `@require` on specific values by mistake.
> 
> Tested with:
> 
>  @requires vm.opt.x.Xlog == "NONEMPTY_TEST_SENTINEL"
>  @requires vm.opt.x.Xlog == "NONEMPTY_TEST_SENTINELXXX"
>  @requires vm.opt.x.Xms == "3g"
> 
> and
> 
> JAVA_OPTIONS=-Xms3g -Xms4g
> JAVA_OPTIONS=-Xms4g -Xms3g
> JAVA_OPTIONS=-Xlog:gc* -Xlog:gc*
> ``` 
> 
> Running tier1

Hi again and sorry for taking so much time. I have been thinking about this for a while, and done some code search inside jtreg etc. I have not really come to a conclusion, but let me try to summarize some of it.

First I want to say that the idea (in the beginning) was not to test for the final value to use but to test that certain flags does not collide/conflict with flags added by the test case. For example, the different flags that chooses a gc collides with each other, and I wanted to make similar checks for short options. I was about to try to change lots of gc test cases to use test APIs that propagates VM flags and I thought I would need that functionality. It does not help me to check the *final* vm flag values if the flags have conflicted before that. It also somewhat irritates me that jtreg has a mechanism to test for only `-XX` flags.

*However*, after this review and after starting to look at certain flags, it seems that it is in /general/ alright to combine flags that obviously conflicts. There seems to be no problem to tell java to use the interpreter and then later to tell it to use the compiler (quite different from telling it to use serial gc followed by parallel that is not allowed). Another thing I have discovered is that it seems to me that vm flags are *prepended* and not appended when using `@run` and when spawning a new test vm using `createTestJavaProcessBuilder`. It was the opposite of what I would have guessed. *It could be that these two observations make it easy enough to skip require flags* and just rely on that user flags are prepended and that test flags are appended and will *override*. If it is also the case that we can mix and match all short flags (*I need your input on this*), it might make it much easier to convert test cases.

I could remove the short flag detection in VMProps, but I would not be happy if I later see that certain of these flags *do* conflict. It might also be that it is good, for other reasons, to test against these flags with `@require` lines. It is also an unfortunate consequence that this behaviour of prepending vm flags that it also makes it extremely hard to know if vm flags will be active in a test case (lets test to see if this test case works with 2 bytes heap --- ooh, it does --- because the test case sets the heap size as well and it overrides). But I digress.

I am willing to remove parsing of short flags if you know that it will not be useful; I think it might be better to just fix the bug as I suspect that this functionality is useful. I also want to say that I am a bit conflicted and that I am not really sure, I do like to remove not needed code. Feedback on if/how short flag conflicts would be valuable for me. Feedback on whether I have understood the prepending of vm flags in both jtreg as well as in our test framework correctly would be welcomed as well.

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

PR Comment: https://git.openjdk.org/jdk/pull/16824#issuecomment-1850676602