RFR: 8252406: Introduce Thread::as_Java_thread() convenience function [v4]

David Holmes david.holmes at oracle.com
Tue Sep 8 10:06:02 UTC 2020


Just to be clear please wait for v5 to appear before reviewing.

Thanks,
David

On 8/09/2020 7:34 pm, David Holmes 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:
> 
>    This reverts commit 689951708fccd0073873e645bf683bc34b75a772
>    This reverts commit da70f8047d3f9c8af9d3eee3b14c71122c5220a0.
>    
>    Reverting v3 and v2 so that we can take a simpler approach that touches far fewer files.
> 
> -------------
> 
> Changes:
>    - all: https://git.openjdk.java.net/jdk/pull/37/files
>    - new: https://git.openjdk.java.net/jdk/pull/37/files/68995170..202af760
> 
> Webrevs:
>   - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=37&range=03
>   - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=37&range=02-03
> 
>    Stats: 110 lines in 40 files changed: 28 ins; 45 del; 37 mod
>    Patch: https://git.openjdk.java.net/jdk/pull/37.diff
>    Fetch: git fetch https://git.openjdk.java.net/jdk pull/37/head:pull/37
> 
> PR: https://git.openjdk.java.net/jdk/pull/37
> 


More information about the serviceability-dev mailing list