RFR: 8297168: Provide a bulk OopHandle release mechanism with the ServiceThread

[Robbin Ehn]

On Mon, 21 Nov 2022 01:56:34 GMT, David Holmes <dholmes at openjdk.org> wrote:

> When a `JavaThread` terminates it has to release all `OopHandles` that it uses. This can't be done by the thread itself due to timing issues, so it is handed-off to the `ServiceThread` to do it - ref [JDK-8244997](https://bugs.openjdk.org/browse/JDK-8244997).
> 
> Initially there was only one `OopHandle` to handle - that of the `threadObj`, but since Loom there are another 3 `OopHandles` to process. The existing logic does the hand-off one `OopHandle` at a time but that is a potential synchronization bottleneck because each hand-off acquires the `ServiceLock`, enqueues the `OopHandle`, issues a notify to (potentially) wakeup the `ServiceThread` and then releases the `ServiceLock`. This can lead to high contention on the `ServiceLock` and also bad scheduling interactions with the `ServiceThread`.
> 
> This PR contains two commits. The first simply changes the API so that we pass the target `JavaThread` so that all 4  `OopHandles` can be extracted in one call. The second commit looks at streamlining things further by consolidating  into a single `OopHandleList` instance.
> 
> As `ServiceThread is a subclass of `JavaThread` I wasn't concerned about it having detailed knowledge of the JavaThread implementation.
> 
> Testing:
>   - tiers 1-3
>   - checked still no memory leak (ref [JDK-8296463](https://bugs.openjdk.org/browse/JDK-8296463))
> 
> Thanks.

Looks fine!

src/hotspot/share/runtime/serviceThread.cpp line 71:

> 69:   OopHandleList(OopHandleList* next) : _next(next), _index(0) {}
> 70:   void add(OopHandle h) {
> 71:     assert(_index < _count, "too many additions");

I think this should be a guarantee.

src/hotspot/share/runtime/serviceThread.cpp line 76:

> 74:   ~OopHandleList() {
> 75:     assert(_index == _count, "usage error");
> 76:     for (int i = 0; i < _count; i++) {

Even if they should be the same I think this should be _index.
Since then the code will work fine just adding 3 items (disregarding the asserts).

-------------

Marked as reviewed by rehn (Reviewer).

PR: https://git.openjdk.org/jdk/pull/11254