Sponsor request: Always pass java.library.path when running micro benchmarks

[Claes Redestad]

Hi Jorn,

looks reasonable to me, and I can sponsor. A review from one of the
build maintainers would be appreciated.

/Claes

On 2020-06-03 22:01, Jorn Vernee wrote:
> Hi,
> 
> Please sponsor the following patch that correctly passes 
> java.library.path when running micro benchmarks [1].
> 
> As a little bit of background; previously the java.library.path was 
> appended to $1_MICRO_JAVA_OPTIONS and this was passed to the java VM 
> running the JMH jar. But, the problem with this is that the path is not 
> passed on to the forks that are created by JMH, so it only works when 
> running without forks.
> 
> As a fix for this, the java.library.path was instead appended to 
> JMH_JVM_ARGS which is ultimately passed to JMH using the -jvmArgs 
> option, which will make sure it is passed to the forks created by JMH as 
> well.
> 
> Unfortunately, it was moved into the same if block that checks whether 
> either MICRO_VM_OPTIONS or MICRO_JAVA_OPTIONS was set on the command 
> line, before appending their values to JMH_JVM_ARGS as well. This means 
> that if MICRO_VM_OPTIONS or MICRO_JAVA_OPTIONS are not set, 
> java.library.path will not be passed to JMH either. This was not a 
> problem in our use case since we needed to set VM_OPTIONS for the 
> benchmark any ways. But it should still be fixed to make benchmarks that 
> don't use VM_OPTIONS, but _do_ use native libraries work, and to avoid a 
> confusing linkage error if the VM_OPTIONS argument is forgotten.
> 
> As a fix, I now unconditionally set $1_JMH_JVM_ARGS to include 
> java.library.path, and then conditionally append MICRO_VM_OPTIONS or 
> MICRO_JAVA_OPTIONS if they were set. Note that I also added the '$1_' 
> prefix to JMH_JVM_ARGS to make it local to the current macro expansion 
> (it seems like this was forgotten before).
> 
> Testing: verifying that with and without setting VM_OPTIONS, 
> java.library.path is set when running the benchmarks.
> 
> Thanks,
> Jorn
> 
> [1] :
> 
> diff --git a/make/RunTests.gmk b/make/RunTests.gmk
> index ccd9632ad4f..805de4dd785 100644
> --- a/make/RunTests.gmk
> +++ b/make/RunTests.gmk
> @@ -687,13 +687,15 @@ define SetupRunMicroTestBody
>       $1_MICRO_BASIC_OPTIONS += -rff 
> $$($1_TEST_RESULTS_DIR)/jmh-result.$(MICRO_RESULTS_FORMAT)
>     endif
> 
> +  # Set library path for native dependencies
> +  $1_JMH_JVM_ARGS := -Djava.library.path=$$(TEST_IMAGE_DIR)/micro/native
> +
>     ifneq ($$(MICRO_VM_OPTIONS)$$(MICRO_JAVA_OPTIONS), )
> -    JMH_JVM_ARGS := $$(MICRO_VM_OPTIONS) $$(MICRO_JAVA_OPTIONS)
> -    # Set library path for native dependencies
> -    JMH_JVM_ARGS += -Djava.library.path=$$(TEST_IMAGE_DIR)/micro/native
> -    $1_MICRO_VM_OPTIONS := -jvmArgs $(call ShellQuote,$$(JMH_JVM_ARGS))
> +    $1_JMH_JVM_ARGS += $$(MICRO_VM_OPTIONS) $$(MICRO_JAVA_OPTIONS)
>     endif
> 
> +  $1_MICRO_VM_OPTIONS := -jvmArgs $(call ShellQuote,$$($1_JMH_JVM_ARGS))
> +
>     ifneq ($$(MICRO_ITER), )
>       $1_MICRO_ITER := -i $$(MICRO_ITER)
>     endif
>