RFR: 8212107: VMThread issues and cleanup

Daniel D.Daugherty dcubed at openjdk.java.net
Fri Sep 18 21:40:40 UTC 2020


On Thu, 17 Sep 2020 19:58:02 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

> We simplify the vmThread by removing the queue and refactor the the main loop.
> This solves the issues listed:
> - It can create an extra safepoint directly after a safepoint.
> - It's not safe for a non-JavaThread to add safepoint to queue while GC do oops do.
> - The exposure of the vm operation is dangerous if it's a handshake.
> - The code is a hornets nest with the repetition of checks and branches
> 
> Passes t1-8, and a benchmark run.
> 
> If you want a smaller diff the commits contains the incremental progress and each commit passed t1.

I probably should have waited to review this after all of Aleksey's
comments were resolved. I'm gonna have to take a look at
src/hotspot/share/runtime/vmThread.cpp again via a webrev;
it's just too hard to review via this snippet UI.

I'll re-review after all of Aleksey's changes are done.

src/hotspot/share/runtime/vmThread.hpp line 115:

> 113:   // Performance measurement
> 114:   static PerfCounter* perf_accumulated_vm_operation_time()
> 115:     { return _perf_accumulated_vm_operation_time; }

Why was this broken into two lines? If keeping that, then
the '{' belong on L114 at the end and not on L115.

src/hotspot/share/runtime/vmThread.cpp line 240:

> 238:   {
> 239:     MonitorLocker ml(_terminate_lock, Mutex::_no_safepoint_check_flag);
> 240:     while(!VMThread::is_terminated()) {

Not your bug, but can you add a space before the '('?

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

PR: https://git.openjdk.java.net/jdk/pull/228


More information about the hotspot-dev mailing list