RFR: 8358573: CompileBroker fails with "expect failure reason" assert with -XX:-InstallMethods [v2]

[Benoît Maillard]

> This PR prevents from hitting an assert when disabling method installation at the end of a successful compilation with the `-XX:-InstallMethods` flag. Previously `CompileBroker` failed to mark the `CompileTask` as complete when using this flag.
> 
> ## Analysis
> 
> We can see that the assert is triggered in `CompileBroker::invoke_compiler_on_method`:
> 
> ```c++
>     if (!ci_env.failing() && !task->is_success()) {
>       assert(ci_env.failure_reason() != nullptr, "expect failure reason");
>       assert(false, "compiler should always document failure: %s", ci_env.failure_reason());
>       // The compiler elected, without comment, not to register a result.
>       // Do not attempt further compilations of this method.
>       ci_env.record_method_not_compilable("compile failed");
>     }
> 
> 
> The `task->is_success()` call accesses the private `_is_success` field. 
> This field is modified in `CompileTask::mark_success`.
> 
> By setting a breakpoint there, and execute the program without `-XX:-InstallMethods`, we get the following stacktrace:
> 
> CompileTask::mark_success compileTask.hpp:185
> nmethod::post_compiled_method nmethod.cpp:2212
> ciEnv::register_method ciEnv.cpp:1127
> Compilation::install_code c1_Compilation.cpp:425
> Compilation::compile_method c1_Compilation.cpp:488
> Compilation::Compilation c1_Compilation.cpp:609
> Compiler::compile_method c1_Compiler.cpp:262
> CompileBroker::invoke_compiler_on_method compileBroker.cpp:2324
> CompileBroker::compiler_thread_loop compileBroker.cpp:1968
> CompilerThread::thread_entry compilerThread.cpp:67
> JavaThread::thread_main_inner javaThread.cpp:773
> JavaThread::run javaThread.cpp:758
> Thread::call_run thread.cpp:243
> thread_native_entry os_linux.cpp:868
> 
> 
> We go up the stacktrace and see that in `Compilation::compile_method` we have:
> 
> ```c++
>   if (should_install_code()) {
>     // install code
>     PhaseTraceTime timeit(_t_codeinstall);
>     install_code(frame_size);
>   }
> 
> 
> If we do not install methods after compilation, the code path that marks the success is never executed
> and therefore results in hitting the assert.
> 
> ### Fix
> <del>We simply mark the task as complete when `should_install_code()` evaluates to `false` in the block code above.</del>
> After careful consideration, it was decided to simply get rid of the `-XX:-InstallMethods` flag.
> 
> ### Testing
> - [x] [GitHub Actions](https://github.com/benoitmaillard/jdk/actions?query=branch%3AJDK-8358573)
> - [x] tier1-3, plus some internal testing
> - [x] Added a test that starts the VM with the `-XX:-InstallMethods` flag
> 
> ...

Benoît Maillard has updated the pull request incrementally with three additional commits since the last revision:

 - 8358573: get rid of InstallMethods flags completely
 - Revert "8358573: Add missing task success notification"
   
   This reverts commit cd91c7c06ba05aba3500b95ba1317539363aa63c.
 - Revert "8358573: Add test for -XX:-InstallMethods"
   
   This reverts commit 6eab84718c3b60c2585bc2711c4bc8144472975b.

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/26310/files
  - new: https://git.openjdk.org/jdk/pull/26310/files/6eab8471..2da73a5d

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=26310&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=26310&range=00-01

  Stats: 54 lines in 4 files changed: 0 ins; 53 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/26310.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/26310/head:pull/26310

PR: https://git.openjdk.org/jdk/pull/26310