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
