Thanks for the review Ioi!
David
On 14/05/2021 3:03 am, Ioi Lam wrote:
On Thu, 13 May 2021 04:59:11 GMT, David Holmes <[email protected]> wrote:
Our code is littered with API's that take, or manifest, a Thread* and then assert/guarantee that it
must be a JavaThread, rather than taking/manifesting a JavaThread in the first place. The main
reason for this is that the TRAPS macro, used in relation to exception generation and processing,
is declared as "Thread* THREAD". In practice only JavaThreads that can execute Java code
can generate arbitrary exceptions, because it requires executing Java code. In other places we can
get away with other kinds of threads "throwing" exceptions because they are only
pre-allocated instances that require no Java code execution (e.g. OOME), or when executed by a
non-JavaThread the code actually by-passes the logic would throw an exception. Such code also
eventually clears the exception and reports the OOM by some other means. We have been progressively
untangling these code paths and modifying TRAPS/CHECK usage so that only JavaThreads will call
TRAPS methods and throw exceptions. Having done t
hat pre-work we are now ready to convert TRAPS to be "JavaThread* THREAD" and
that is what this change does. The resulting changes are largely mechanical:
- declarations of "Thread* THREAD" become "JavaThread* THREAD" (where THREAD is
needed for exceptions, otherwise THREAD is renamed to current)
- methods that took a Thread* parameter that was always THREAD, now take a
JavaThread* param
- Manifestations of Thread::current() become JavaThread::current()
- THREAD->as_Java_thread() becomes just THREAD
- THREAD->is_Java_thread() is reworked so that THREAD is "current"
There are still places where a CompilerThread (which is a JavaThread but may
not be able to execute Java code) calls a TRAPS function (primarily where OOME
is possible). Fixing that would be a future RFE and may not be possible without
reworking a lot of the allocation code paths.
Testing:
- tiers 1-8 on Linux-x64
- all builds in tiers 1-4
- tiers 1-3 on all platforms
Thanks,
David
David Holmes has updated the pull request with a new target base due to a merge
or a rebase. The pull request now contains five commits:
- Update after merge with master
- Merge branch 'master' into 8252685-JavaThread
- Review feedback from Serguei
- Merge
- 8252685: APIs that require JavaThread should take JavaThread arguments
LGTM
-------------
Marked as reviewed by iklam (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/3877
