On Tue, 8 Sep 2020 04:38:43 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> This is a rather large but generally simple cleanup. >> >> We use a lot of raw C-style casts to "(JavaThread*)" typically after >> checking "thread->is_Java_thread()". To simplify >> this pattern, and make the code look somewhat cleaner we introduce a >> convenience function Thread::as_Java_thread(), >> which asserts is_Java_thread() and returns "this" cast to "JavaThread*". A >> const version, as_const_Java_thread(), was >> also added to allow use in cases where we start with "const Thread *". >> Summary of changes: >> - changed raw casts to use as_Java_thread() >> - removed redundant checks of is_Java_thread() as it is now done in >> as_Java_thread() >> - Removed checks that are checking the type system e.g. >> void foo(JavaThread* t) { >> assert(t->is_Java_thread(), "must be") >> the only way the assert can fire is if someone deliberately bypasses the >> type system, and if we are going to worry >> about that then we would need asserts like this on every method that expects >> a JavaThread! The right place for the >> assertion is at the head of any such call chain. >> - Removed asserts that are already guaranteed in the caller. >> - Use JavaThread::current() where appropriate. >> - Use pre-existing "thread" variable where available rather than casting >> THREAD >> >> Change locations found by grepping for variations of the cast syntax >> "(JavaThread*)" - it is possible some may have >> been missed. >> Testing: tiers 1-3 >> >> Thanks, >> David > > David Holmes has updated the pull request incrementally with one additional > commit since the last revision: > > More missing includes of thread.inline.hpp > > The fanout for this change is now quite large. Shenandoah parts still look good. Formally approving. ------------- Marked as reviewed by shade (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/37