RFR(s): 8214181: safepoint header cleanup

[David Holmes]

Hi Robbin,

On 22/11/2018 12:17 am, Robbin Ehn wrote:
> Hi all, please review.
> 
> A cleanup of unused headers in safepoint.hpp, which have some minor 
> collateral effects.

I see a lot of changes here which seem unrelated to cleaning up 
safepoint.hpp per-se. The * changes are a distraction.

src/hotspot/share/gc/g1/g1HeapVerifier.cpp

I see no include of safepoint.hpp that would have previously caused 
code/nmethod.hpp to be included - what was the path here?

Ditto for src/hotspot/share/memory/iterator.cpp

---
src/hotspot/share/runtime/thread.hpp

How does code/compiledMethod.hpp relate to safepoint.hpp?

Moving stuff to .inline.hpp is fine, but seems unrelated to safepoint 
header cleanup.

With the header file changes I have to wonder how many of those advance 
class definitions are actually needed. The

  61 class ThreadSafepointState;

wasn't needed before, but now it is as you've removed the include.

---

safepoint.hpp:

I don't think I'll ever figure out how include headers are supposed to 
be handled. I see so many types being used in safepoint.hpp for which 
there is no include for the header defining those types! :(

That said I don't think we should be making implicit dependencies worse, 
so as safepoint.hpp calls os::javaTimeMillis() I think the os.hpp 
include should remain.

Thanks,
David

> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8214181
> 
> Webrev:
> http://cr.openjdk.java.net/~rehn/8214181/webrev/index.html
> Passes t1, locally built without precompiled headers.
> 
> Thanks, Robbin