RFR(S): 8233193: Incorrect bailout from possibly_add_compiler_threads

[David Holmes]

Hi Martin,

On 15/11/2019 2:18 am, Doerr, Martin wrote:
> Hi,
> 
> I'd like to cleanup exception handling in CompileBroker a little bit.
> 
> Here's my proposal:
> 
> - Use THREAD instead of CHECK where no exceptions get thrown.

That's fine but you also replaced TRAPS with "Thread* THREAD" which is 
pointless given:

#define TRAPS  Thread* THREAD

> - Remove unused preload_classes.

Ok

> - make_thread: Rename thread to new_thread to avoid confusion with 
> THREAD (the current compiler thread).

Ok

> - possibly_add_compiler_threads: Remove usage of EXCEPTION_MARK + CHECK 
> because this functions is not supposed to kill the VM on exceptions. Add 
> assertion to caller.

Ok

> Webrev:
> 
> http://cr.openjdk.java.net/~mdoerr/8233193_CompileBroker/webrev.00/
> 
> @David:
> 
> You didn't like usage of the CHECK macro in the initialization 
> functions, but I think they are ok.
> 
> Not very nice to read, but the behavior looks ok to me.
> 
> At least, I didn't find a better replacement for them. Maybe you have a 
> proposal?

May I suggest a comment then:

  844 void CompileBroker::init_compiler_sweeper_threads() {
        // Ensure any exceptions lead to vm_exit_during_initialization
  845   EXCEPTION_MARK;

Thanks,
David
-----

> Best regards,
> 
> Martin
>