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
