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

[Erik Joelsson]

Looks good.

/Erik

On 2020-06-03 13: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
>