Avoid synthetic methods in Recycler (#9736)

Motivation

Currently the visibility of the various Recycler inner classes and their
fields isn't optimal. Some private members are accessed by other classes
resulting in synthetic methods, and other non-private classes/members
are only accessed privately and so can be made private.

Modifications

- Increase/reduce visibility of various fields/methods/classes within
Recycler
- Have WeakOrderQueue extend WeakReference<Thread> to eliminate the
owner field
- Change local DefaultHandle var to DefaultHandle<?> to avoid raw type
compiler warning

Result

Tidier code, fewer implicit methods on hot paths (reducing inlining
depths)
This commit is contained in:
Nick Hill 2019-10-31 03:22:05 -07:00 committed by Norman Maurer
parent 3501f82ef2
commit 7c543a48e5

View File

@ -194,14 +194,14 @@ public abstract class Recycler<T> {
public interface Handle<T> extends ObjectPool.Handle<T> { }
static final class DefaultHandle<T> implements Handle<T> {
private int lastRecycledId;
private int recycleId;
private static final class DefaultHandle<T> implements Handle<T> {
int lastRecycledId;
int recycleId;
boolean hasBeenRecycled;
private Stack<?> stack;
private Object value;
Stack<?> stack;
Object value;
DefaultHandle(Stack<?> stack) {
this.stack = stack;
@ -232,21 +232,21 @@ public abstract class Recycler<T> {
// a queue that makes only moderate guarantees about visibility: items are seen in the correct order,
// but we aren't absolutely guaranteed to ever see anything at all, thereby keeping the queue cheap to maintain
private static final class WeakOrderQueue {
private static final class WeakOrderQueue extends WeakReference<Thread> {
static final WeakOrderQueue DUMMY = new WeakOrderQueue();
// Let Link extend AtomicInteger for intrinsics. The Link itself will be used as writerIndex.
@SuppressWarnings("serial")
static final class Link extends AtomicInteger {
private final DefaultHandle<?>[] elements = new DefaultHandle[LINK_CAPACITY];
final DefaultHandle<?>[] elements = new DefaultHandle[LINK_CAPACITY];
private int readIndex;
int readIndex;
Link next;
}
// Its important this does not hold any reference to either Stack or WeakOrderQueue.
static final class Head {
private static final class Head {
private final AtomicInteger availableSharedCapacity;
Link link;
@ -302,15 +302,15 @@ public abstract class Recycler<T> {
private Link tail;
// pointer to another queue of delayed items for the same stack
private WeakOrderQueue next;
private final WeakReference<Thread> owner;
private final int id = ID_GENERATOR.getAndIncrement();
private WeakOrderQueue() {
owner = null;
super(null);
head = new Head(null);
}
private WeakOrderQueue(Stack<?> stack, Thread thread) {
super(thread);
tail = new Link();
// Its important that we not store the Stack itself in the WeakOrderQueue as the Stack also is used in
@ -318,10 +318,9 @@ public abstract class Recycler<T> {
// Stack itself GCed.
head = new Head(stack.availableSharedCapacity);
head.link = tail;
owner = new WeakReference<>(thread);
}
static WeakOrderQueue newQueue(Stack<?> stack, Thread thread) {
private static WeakOrderQueue newQueue(Stack<?> stack, Thread thread) {
final WeakOrderQueue queue = new WeakOrderQueue(stack, thread);
// Done outside of the constructor to ensure WeakOrderQueue.this does not escape the constructor and so
// may be accessed while its still constructed.
@ -330,7 +329,11 @@ public abstract class Recycler<T> {
return queue;
}
private void setNext(WeakOrderQueue next) {
WeakOrderQueue getNext() {
return next;
}
void setNext(WeakOrderQueue next) {
assert next != this;
this.next = next;
}
@ -411,7 +414,7 @@ public abstract class Recycler<T> {
final DefaultHandle[] dstElems = dst.elements;
int newDstSize = dstSize;
for (int i = srcStart; i < srcEnd; i++) {
DefaultHandle element = srcElems[i];
DefaultHandle<?> element = srcElems[i];
if (element.recycleId == 0) {
element.recycleId = element.lastRecycledId;
} else if (element.recycleId != element.lastRecycledId) {
@ -446,7 +449,7 @@ public abstract class Recycler<T> {
}
}
static final class Stack<T> {
private static final class Stack<T> {
// we keep a queue of per-thread queues, which is appended to once only, each time a new thread other
// than the stack owner recycles: when we run out of items in our stack we iterate this collection
@ -462,12 +465,12 @@ public abstract class Recycler<T> {
// it in a timely manner).
final WeakReference<Thread> threadRef;
final AtomicInteger availableSharedCapacity;
final int maxDelayedQueues;
private final int maxDelayedQueues;
private final int maxCapacity;
private final int ratioMask;
private DefaultHandle<?>[] elements;
private int size;
DefaultHandle<?>[] elements;
int size;
private int handleRecycleCount = -1; // Start with -1 so the first one will be recycled.
private WeakOrderQueue cursor, prev;
private volatile WeakOrderQueue head;
@ -533,7 +536,7 @@ public abstract class Recycler<T> {
return ret;
}
boolean scavenge() {
private boolean scavenge() {
// continue an existing scavenge, if any
if (scavengeSome()) {
return true;
@ -545,7 +548,7 @@ public abstract class Recycler<T> {
return false;
}
boolean scavengeSome() {
private boolean scavengeSome() {
WeakOrderQueue prev;
WeakOrderQueue cursor = this.cursor;
if (cursor == null) {
@ -564,8 +567,8 @@ public abstract class Recycler<T> {
success = true;
break;
}
WeakOrderQueue next = cursor.next;
if (cursor.owner.get() == null) {
WeakOrderQueue next = cursor.getNext();
if (cursor.get() == null) {
// If the thread associated with the queue is gone, unlink it, after
// performing a volatile read to confirm there is no data left to collect.
// We never unlink the first queue, as we don't want to synchronize on updating the head.