RFR (XL) 8173715: Remove FlatProfiler

[David Holmes]

Hi Gerard,

On 17/08/2017 3:48 AM, Gerard Ziemski wrote:
> hi all,
> 
> (this is hotspot part of the change, the jdk part is being reviewed on jdk10-dev at openjdk.java.net)
> 
> The FlatProfiler (i.e. -Xprof) has been deprecated for a while, and now it’s the time to remove it.

That is fine for removing the support, but shouldn't the actual -Xprof 
flab be obsoleted before being actually removed? Though this is -X not 
-XX so I'm not sure what the launcher protocol is for this. ??
> The changes are XL, but straightforward.
> 
> If anyone knows of any other remnants that should still be removed, please let me know.
> 
> issue:  https://bugs.openjdk.java.net/browse/JDK-8173715
> webrev: http://cr.openjdk.java.net/~gziemski/8173715_rev1_hotspot

There are a few places where you have removed reference to the flat 
profiler, or -Xprof, but you haven't actually changed the code that only 
seemed to have existed to support the flat-profiler e.g.:

src/cpu/sparc/vm/macroAssembler_sparc.cpp

-    save_frame_and_mov(0, Lmethod, Lmethod);   // to avoid clobbering 
O0 (and propagate Lmethod for -Xprof)
+    save_frame_and_mov(0, Lmethod, Lmethod);   // to avoid clobbering 
O0 (and propagate Lmethod)

Are we still propagating Lmethod here? If so why? Ditto for other places 
in the file.

In: src/share/vm/runtime/thread.cpp

-    // eventually timeout, but that takes time. Making this wait a
-    // suspend-equivalent condition solves that timeout problem.
-    //
      Terminator_lock->wait(!Mutex::_no_safepoint_check_flag, 0,
                            Mutex::_as_suspend_equivalent_flag);

So we should no longer need the Mutex::_as_suspend_equivalent_flag here 
(and in another place).

---

I'm a little perplexed by the changes to the os_*.cpp file regarding 
low-level suspend/resume:

-//  within hotspot. Now there is a single use-case for this:
-//    - calling get_thread_pc() on the VMThread by the flat-profiler task
-//      that runs in the watcher thread.
+//  within hotspot. Needed for fetch_frame_from_ucontext(), which is 
used by:
+//    - Forte Analyzer: AsyncGetCallTrace()
+//    - StackBanging: get_frame_at_stack_banging_point()

I don't see any use of low-level suspend/resume with the two cases you 
added ??

---

src/os_cpu/linux_s390/vm/thread_linux_s390.hpp	

  /*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.

That's an incorrect copyright update - should be "2016, 2017,"

---

src/os/solaris/vm/osThread_solaris.hpp

   // ***************************************************************
- // interrupt support.  interrupts (using signals) are used to get
- // the thread context (get_thread_pc), to set the thread context
- // (set_thread_pc), and to implement java.lang.Thread.interrupt.
+ // interrupt support.  interrupts (using signals) are used to
+ // implement java.lang.Thread.interrupt.
   // ***************************************************************

That entire comment block can be deleted. We haven't used signals for 
j.l.T.interrupt for years.

---

Thanks,
David


> passes the following tests:
> 
>   JPRT
>   RBT test_hotspot/test/serviceability/jvmti
>   RBT test_hotspot/test/gc
>   RBT test_hotspot/test/compiler
>   RBT test_hotspot/test/runtime
> 
> Thank you.
>