Skip to content

Commit

Permalink
Defer logger construction, which can be expensive under Android.
Browse files Browse the repository at this point in the history
RELNOTES=`util.concurrent`: Changed our implementations to avoid eagerly initializing loggers during class loading.
PiperOrigin-RevId: 590666126
  • Loading branch information
cpovirk authored and Google Java Core Libraries committed Dec 13, 2023
1 parent 3566041 commit 4fe1df5
Show file tree
Hide file tree
Showing 28 changed files with 334 additions and 182 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.logging.Level;
import java.util.logging.Logger;

/**
* Base class for services that can implement {@link #startUp}, {@link #run} and {@link #shutDown}
Expand All @@ -37,8 +36,7 @@
@J2ktIncompatible
@ElementTypesAreNonnullByDefault
public abstract class AbstractExecutionThreadService implements Service {
private static final Logger logger =
Logger.getLogger(AbstractExecutionThreadService.class.getName());
private static final LazyLogger logger = new LazyLogger(AbstractExecutionThreadService.class);

/* use AbstractService for state management */
private final Service delegate =
Expand All @@ -65,10 +63,12 @@ protected final void doStart() {
// TODO(lukes): if guava ever moves to java7, this would be a good
// candidate for a suppressed exception, or maybe we could generalize
// Closer.Suppressor
logger.log(
Level.WARNING,
"Error while attempting to shut down the service after failure.",
ignored);
logger
.get()
.log(
Level.WARNING,
"Error while attempting to shut down the service after failure.",
ignored);
}
notifyFailed(t);
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
import java.util.concurrent.locks.LockSupport;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.CheckForNull;
import org.checkerframework.checker.nullness.qual.Nullable;

Expand Down Expand Up @@ -141,11 +140,7 @@ public final boolean cancel(boolean mayInterruptIfRunning) {
}
}

// Logger to log exceptions caught when running listeners.
// Holder class to delay initialization, for performance reasons.
private static final class LoggerHolder {
static final Logger log = Logger.getLogger(AbstractFuture.class.getName());
}
static final LazyLogger log = new LazyLogger(AbstractFuture.class);

// A heuristic for timed gets. If the remaining timeout is less than this, spin instead of
// blocking. This value is what AbstractQueuedSynchronizer uses.
Expand Down Expand Up @@ -192,9 +187,12 @@ private static final class LoggerHolder {
// Log after all static init is finished; if an installed logger uses any Futures methods, it
// shouldn't break in cases where reflection is missing/broken.
if (thrownAtomicReferenceFieldUpdaterFailure != null) {
LoggerHolder.log.log(Level.SEVERE, "UnsafeAtomicHelper is broken!", thrownUnsafeFailure);
LoggerHolder.log.log(
Level.SEVERE, "SafeAtomicHelper is broken!", thrownAtomicReferenceFieldUpdaterFailure);
log.get().log(Level.SEVERE, "UnsafeAtomicHelper is broken!", thrownUnsafeFailure);
log.get()
.log(
Level.SEVERE,
"SafeAtomicHelper is broken!",
thrownAtomicReferenceFieldUpdaterFailure);
}
}

Expand Down Expand Up @@ -1301,10 +1299,14 @@ private static void executeListener(Runnable runnable, Executor executor) {
// Log it and keep going -- bad runnable and/or executor. Don't punish the other runnables if
// we're given a bad one. We only catch RuntimeException because we want Errors to propagate
// up.
LoggerHolder.log.log(
Level.SEVERE,
"RuntimeException while executing runnable " + runnable + " with executor " + executor,
e);
log.get()
.log(
Level.SEVERE,
"RuntimeException while executing runnable "
+ runnable
+ " with executor "
+ executor,
e);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import java.util.concurrent.TimeoutException;
import java.util.concurrent.locks.ReentrantLock;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.CheckForNull;
import org.checkerframework.checker.nullness.qual.Nullable;

Expand Down Expand Up @@ -102,7 +101,7 @@
@J2ktIncompatible
@ElementTypesAreNonnullByDefault
public abstract class AbstractScheduledService implements Service {
private static final Logger logger = Logger.getLogger(AbstractScheduledService.class.getName());
private static final LazyLogger logger = new LazyLogger(AbstractScheduledService.class);

/**
* A scheduler defines the policy for how the {@link AbstractScheduledService} should run its
Expand Down Expand Up @@ -209,10 +208,12 @@ public void run() {
shutDown();
} catch (Exception ignored) {
restoreInterruptIfIsInterruptedException(ignored);
logger.log(
Level.WARNING,
"Error while attempting to shut down the service after failure.",
ignored);
logger
.get()
.log(
Level.WARNING,
"Error while attempting to shut down the service after failure.",
ignored);
}
notifyFailed(t);
// requireNonNull is safe now, just as it was above.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
import java.util.logging.Logger;
import javax.annotation.CheckForNull;
import org.checkerframework.checker.nullness.qual.Nullable;

Expand All @@ -44,7 +43,7 @@
@ElementTypesAreNonnullByDefault
abstract class AggregateFuture<InputT extends @Nullable Object, OutputT extends @Nullable Object>
extends AggregateFutureState<OutputT> {
private static final Logger logger = Logger.getLogger(AggregateFuture.class.getName());
private static final LazyLogger logger = new LazyLogger(AggregateFuture.class);

/**
* The input futures. After {@link #init}, this field is read only by {@link #afterDone()} (to
Expand Down Expand Up @@ -230,7 +229,7 @@ private static void log(Throwable throwable) {
(throwable instanceof Error)
? "Input Future failed with Error"
: "Got more than one input Future failure. Logging failures after the first";
logger.log(SEVERE, message, throwable);
logger.get().log(SEVERE, message, throwable);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import java.util.concurrent.atomic.AtomicIntegerFieldUpdater;
import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.CheckForNull;
import org.checkerframework.checker.nullness.qual.Nullable;

Expand All @@ -51,7 +50,7 @@ abstract class AggregateFutureState<OutputT extends @Nullable Object>

private static final AtomicHelper ATOMIC_HELPER;

private static final Logger log = Logger.getLogger(AggregateFutureState.class.getName());
private static final LazyLogger log = new LazyLogger(AggregateFutureState.class);

static {
AtomicHelper helper;
Expand All @@ -73,7 +72,7 @@ abstract class AggregateFutureState<OutputT extends @Nullable Object>
// Log after all static init is finished; if an installed logger uses any Futures methods, it
// shouldn't break in cases where reflection is missing/broken.
if (thrownReflectionFailure != null) {
log.log(Level.SEVERE, "SafeAtomicHelper is broken!", thrownReflectionFailure);
log.get().log(Level.SEVERE, "SafeAtomicHelper is broken!", thrownReflectionFailure);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@
import java.util.concurrent.Future;
import java.util.concurrent.RejectedExecutionException;
import java.util.concurrent.atomic.AtomicReference;
import java.util.logging.Logger;
import javax.annotation.CheckForNull;
import org.checkerframework.checker.nullness.qual.Nullable;

Expand Down Expand Up @@ -196,7 +195,7 @@
// TODO(dpb): GWT compatibility.
public final class ClosingFuture<V extends @Nullable Object> {

private static final Logger logger = Logger.getLogger(ClosingFuture.class.getName());
private static final LazyLogger logger = new LazyLogger(ClosingFuture.class);

/**
* An object that can capture objects to be closed later, when a {@link ClosingFuture} pipeline is
Expand Down Expand Up @@ -1018,7 +1017,7 @@ public String toString() {
*/
public FluentFuture<V> finishToFuture() {
if (compareAndUpdateState(OPEN, WILL_CLOSE)) {
logger.log(FINER, "will close {0}", this);
logger.get().log(FINER, "will close {0}", this);
future.addListener(
new Runnable() {
@Override
Expand Down Expand Up @@ -1118,7 +1117,7 @@ public void run() {
*/
@CanIgnoreReturnValue
public boolean cancel(boolean mayInterruptIfRunning) {
logger.log(FINER, "cancelling {0}", this);
logger.get().log(FINER, "cancelling {0}", this);
boolean cancelled = future.cancel(mayInterruptIfRunning);
if (cancelled) {
close();
Expand All @@ -1127,7 +1126,7 @@ public boolean cancel(boolean mayInterruptIfRunning) {
}

private void close() {
logger.log(FINER, "closing {0}", this);
logger.get().log(FINER, "closing {0}", this);
closeables.close();
}

Expand Down Expand Up @@ -2131,7 +2130,7 @@ public String toString() {
@Override
protected void finalize() {
if (state.get().equals(OPEN)) {
logger.log(SEVERE, "Uh oh! An open ClosingFuture has leaked and will close: {0}", this);
logger.get().log(SEVERE, "Uh oh! An open ClosingFuture has leaked and will close: {0}", this);
FluentFuture<V> unused = finishToFuture();
}
}
Expand All @@ -2155,13 +2154,17 @@ private static void closeQuietly(@CheckForNull final Closeable closeable, Execut
* that we have to account for sneaky checked exception.
*/
restoreInterruptIfIsInterruptedException(e);
logger.log(WARNING, "thrown by close()", e);
logger.get().log(WARNING, "thrown by close()", e);
}
});
} catch (RejectedExecutionException e) {
if (logger.isLoggable(WARNING)) {
logger.log(
WARNING, String.format("while submitting close to %s; will close inline", executor), e);
if (logger.get().isLoggable(WARNING)) {
logger
.get()
.log(
WARNING,
String.format("while submitting close to %s; will close inline", executor),
e);
}
closeQuietly(closeable, directExecutor());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
import java.util.concurrent.locks.ReentrantLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.CheckForNull;

/**
Expand Down Expand Up @@ -211,7 +210,7 @@ public void handlePotentialDeadlock(PotentialDeadlockException e) {
WARN {
@Override
public void handlePotentialDeadlock(PotentialDeadlockException e) {
logger.log(Level.SEVERE, "Detected potential deadlock", e);
logger.get().log(Level.SEVERE, "Detected potential deadlock", e);
}
},

Expand Down Expand Up @@ -447,7 +446,7 @@ public ReentrantReadWriteLock newReentrantReadWriteLock(E rank, boolean fair) {

//////// Implementation /////////

private static final Logger logger = Logger.getLogger(CycleDetectingLockFactory.class.getName());
private static final LazyLogger logger = new LazyLogger(CycleDetectingLockFactory.class);

final Policy policy;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import com.google.errorprone.annotations.concurrent.GuardedBy;
import java.util.concurrent.Executor;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.CheckForNull;

/**
Expand All @@ -45,7 +44,7 @@
@ElementTypesAreNonnullByDefault
public final class ExecutionList {
/** Logger to log exceptions caught when running runnables. */
private static final Logger log = Logger.getLogger(ExecutionList.class.getName());
private static final LazyLogger log = new LazyLogger(ExecutionList.class);

/**
* The runnable, executor pairs to execute. This acts as a stack threaded through the {@link
Expand Down Expand Up @@ -148,10 +147,14 @@ private static void executeListener(Runnable runnable, Executor executor) {
// Log it and keep going -- bad runnable and/or executor. Don't punish the other runnables if
// we're given a bad one. We only catch RuntimeException because we want Errors to propagate
// up.
log.log(
Level.SEVERE,
"RuntimeException while executing runnable " + runnable + " with executor " + executor,
e);
log.get()
.log(
Level.SEVERE,
"RuntimeException while executing runnable "
+ runnable
+ " with executor "
+ executor,
e);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import java.util.concurrent.Executor;
import java.util.concurrent.TimeUnit;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.CheckForNull;
import org.checkerframework.checker.nullness.qual.Nullable;

Expand All @@ -33,7 +32,7 @@
class ImmediateFuture<V extends @Nullable Object> implements ListenableFuture<V> {
static final ListenableFuture<?> NULL = new ImmediateFuture<@Nullable Object>(null);

private static final Logger log = Logger.getLogger(ImmediateFuture.class.getName());
private static final LazyLogger log = new LazyLogger(ImmediateFuture.class);

@ParametricNullness private final V value;

Expand All @@ -51,10 +50,14 @@ public void addListener(Runnable listener, Executor executor) {
} catch (Exception e) { // sneaky checked exception
// ListenableFuture's contract is that it will not throw unchecked exceptions, so log the bad
// runnable and/or executor and swallow it.
log.log(
Level.SEVERE,
"RuntimeException while executing runnable " + listener + " with executor " + executor,
e);
log.get()
.log(
Level.SEVERE,
"RuntimeException while executing runnable "
+ listener
+ " with executor "
+ executor,
e);
}
}

Expand Down
Loading

0 comments on commit 4fe1df5

Please sign in to comment.