Skip to content

Commit

Permalink
Adjust PhaseState pooling
Browse files Browse the repository at this point in the history
This is a correctness fix as we should not store the states
before they are properly closed and the state has been cleared.

We also didn't clear or return states to the pool that
didn't provide modifiers and could leak resources for
long periods of time until they were used again.

As added bonus we now pool more states and removes some branching.
  • Loading branch information
aromaa committed Apr 15, 2024
1 parent ac14685 commit 4c80359
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -323,22 +323,14 @@ public void close() { // Should never throw an exception
}
final PhaseTracker instance = PhaseTracker.getInstance();
instance.completePhase(this);
if (!this.shouldProvideModifiers()) {
if (this.usedFrame != null) {
this.usedFrame.iterator().forEachRemaining(instance::popCauseFrame);
}
return;
}
if (this.usedFrame == null) {
// So, this part is interesting... Since the used frame is null, that means
// the cause stack manager still has the refernce of this context/phase, we have
// to "pop off" the list.
instance.popFrameMutator(this);
}
if (this.usedFrame != null) {
this.usedFrame.iterator().forEachRemaining(instance::popCauseFrame);
this.usedFrame.clear();
this.usedFrame = null;
} else if (this.shouldProvideModifiers()) {
// So, this part is interesting... Since the used frame is null, that means
// the cause stack manager still has the reference of this context/phase, we have
// to "pop off" the list.
instance.popFrameMutator(this);
}
this.reset();
this.isCompleted = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,34 +43,23 @@ public final C createPhaseContext(final PhaseTracker tracker) {
}

if (tracker == PhaseTracker.SERVER) {
if (this.serverCached != null && !this.serverCached.isCompleted) {
if (this.serverCached != null) {
final C cached = this.serverCached;
this.serverCached = null;
return cached;
}
} else if (tracker == PhaseTracker.CLIENT) {
if (this.clientCached != null && !this.clientCached.isCompleted) {
if (this.clientCached != null) {
final C cached = this.clientCached;
this.clientCached = null;
return cached;
}
}
final @Nullable C peek = tracker.getContextPoolFor(this).pollFirst();
if (peek != null) {
if (tracker == PhaseTracker.SERVER) {
this.serverCached = peek;
} else if (tracker == PhaseTracker.CLIENT) {
this.clientCached = peek;
}
return peek;
}
final C maybeCached = this.createNewContext(tracker);
if (tracker == PhaseTracker.SERVER) {
this.serverCached = maybeCached;
} else if (tracker == PhaseTracker.CLIENT) {
this.clientCached = maybeCached;
}
return maybeCached;
return this.createNewContext(tracker);
}

final void releaseContextFromPool(final C context) {
Expand All @@ -79,25 +68,18 @@ final void releaseContextFromPool(final C context) {
throw new IllegalStateException("Asynchronous Thread Access to PhaseTracker: " + createdTracker);
}
if (createdTracker == PhaseTracker.SERVER) {
if (this.serverCached == context) {
return;
}
if (this.serverCached == null) {
this.serverCached = context;
return;
}
}
if (createdTracker == PhaseTracker.CLIENT) {
if (this.clientCached == context) {
return;
}
else if (createdTracker == PhaseTracker.CLIENT) {
if (this.clientCached == null) {
this.clientCached = context;
return;
}
}
createdTracker.getContextPoolFor(this).push(context);

}

protected abstract C createNewContext(PhaseTracker tracker);
Expand Down

0 comments on commit 4c80359

Please sign in to comment.