Review-Request: Fix of JDK-8034775 neglects to account for non-JIT VMs

[David Holmes]

On 28/04/2014 9:42 PM, Severin Gehwolf wrote:
> Hi,
>
> Bug: JI-9011998 (I don't seem to be able to create JDK bugs)
> Webrev: http://jerboaa.fedorapeople.org/bugs/openjdk/JI-9011998/v1/
>
> The fix for JDK-8034775 introduced a start-up check requiring the number
> of compiler threads to be >= 1, which does not make sense for non-JIT
> VMs such as the zero JVM variant. This causes zero JVMs to fail
> initialization with:
>
> CICompilerCount of 0 is invalid; must be at least 1
> Error: Could not create the Java Virtual Machine.
> Error: A fatal exception has occurred. Program will exit.
>
> This is caused by a wrong start-up check in
> src/share/vm/runtime/arguments.cpp where a minimal value of 1 is
> required no matter the JVM variant.
>
> The proposed fix uses the defined CI_COMPILER_COUNT pre-processor
> constant over a static 1 to pass to verify_min_value(). Since
> CI_COMPILER_COUNT is going to be defined differently for JVM variants it
> will make the lower water mark check correct for all JVM variants.
>
> There was an error in defining CI_COMPILER_COUNT as well. On line 196 in
> src/share/vm/runtime/globals.hpp CI_COMPILER_COUNT is defined to be 0
> (since COMPILER1, COMPILER2 and SHARK are not defined for a Zero build.
> Then on line 201 in src/share/vm/runtime/globals.hpp the "else" branch
> of ifdef COMPILER2 is entered and the earlier definition of
> CI_COMPILER_COUNT (with value 0) overridden to 1.

I don't see that. There are nested if-else branches:

  167 #if !defined(COMPILER1) && !defined(COMPILER2) && !defined(SHARK)
  ...
  196     #define CI_COMPILER_COUNT 0
  197 #else
  198
  199     #ifdef COMPILER2
  200         #define CI_COMPILER_COUNT 2
  201     #else
  202         #define CI_COMPILER_COUNT 1
  203     #endif // COMPILER2
  204
  205 #endif // no compilers

David
-----


> I've amended test/compiler/startup/NumCompilerThreadsCheck.java so as to
> verify that the lower water mark for Zero JVMs is 0.
>
> Thoughts?
>
> Cheers,
> Severin
>