From bf734b9d77ba5dcffd881ba6d13cf8af02abb723 Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Fri, 8 Jun 2018 10:36:13 +0200 Subject: [PATCH] Remove unneeded checks from logDebug methods to check if fine logging is enabled Calling the logVarargs method is expensive since it requires allocating an object array, however in case of the logDebug methods the object array was already allocated when calling the logDebug method, hence checking inside of the logDebug methods if fine logging is enabled to avoid calling the logVarargs method if it's not enabled doesn't help. The custom logDebug methods are still needed to include request IDs, submission IDs etc. In the long this should be done via Flogger tags and the custom logDebug methods should be removed (but we don't have support for Flogger tags yet). Change-Id: Ia1a3048fa3d0ab2fbb801dc278d0d2d61e2c86dc Signed-off-by: Edwin Kempin --- .../google/gerrit/server/git/receive/ReceiveCommits.java | 4 +--- java/com/google/gerrit/server/submit/GitModules.java | 4 +--- java/com/google/gerrit/server/submit/MergeOp.java | 4 +--- .../com/google/gerrit/server/submit/SubmitStrategyOp.java | 4 +--- java/com/google/gerrit/server/submit/SubmoduleOp.java | 4 +--- java/com/google/gerrit/server/submit/TestHelperOp.java | 4 +--- java/com/google/gerrit/server/update/BatchUpdate.java | 4 ++-- .../google/gerrit/server/update/ReviewDbBatchUpdate.java | 8 ++------ 8 files changed, 10 insertions(+), 26 deletions(-) diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java index 87587df08c..9bd4db2d10 100644 --- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java +++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java @@ -3097,9 +3097,7 @@ class ReceiveCommits { } private void logDebug(String msg, Object... args) { - if (logger.atFine().isEnabled()) { - logger.atFine().logVarargs(receiveId + msg, args); - } + logger.atFine().logVarargs(receiveId + msg, args); } private void logWarn(String msg, Throwable t) { diff --git a/java/com/google/gerrit/server/submit/GitModules.java b/java/com/google/gerrit/server/submit/GitModules.java index 5813c5e916..826e737b25 100644 --- a/java/com/google/gerrit/server/submit/GitModules.java +++ b/java/com/google/gerrit/server/submit/GitModules.java @@ -104,8 +104,6 @@ public class GitModules { } private void logDebug(String msg, Object... args) { - if (logger.atFine().isEnabled()) { - logger.atFine().logVarargs(submissionId + msg, args); - } + logger.atFine().logVarargs(submissionId + msg, args); } } diff --git a/java/com/google/gerrit/server/submit/MergeOp.java b/java/com/google/gerrit/server/submit/MergeOp.java index cc3e5a96f5..67b56ea30d 100644 --- a/java/com/google/gerrit/server/submit/MergeOp.java +++ b/java/com/google/gerrit/server/submit/MergeOp.java @@ -937,9 +937,7 @@ public class MergeOp implements AutoCloseable { } private void logDebug(String msg, Object... args) { - if (logger.atFine().isEnabled()) { - logger.atFine().logVarargs(submissionId + msg, args); - } + logger.atFine().logVarargs(submissionId + msg, args); } private void logWarn(String msg, Throwable t) { diff --git a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java index afcdef532c..9472cf8cd0 100644 --- a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java +++ b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java @@ -601,9 +601,7 @@ abstract class SubmitStrategyOp implements BatchUpdateOp { } protected final void logDebug(String msg, Object... args) { - if (logger.atFine().isEnabled()) { - logger.atFine().logVarargs(this.args.submissionId + msg, args); - } + logger.atFine().logVarargs(this.args.submissionId + msg, args); } protected final void logWarn(String msg, Throwable t) { diff --git a/java/com/google/gerrit/server/submit/SubmoduleOp.java b/java/com/google/gerrit/server/submit/SubmoduleOp.java index 7c5e6812e9..4d9c2a6101 100644 --- a/java/com/google/gerrit/server/submit/SubmoduleOp.java +++ b/java/com/google/gerrit/server/submit/SubmoduleOp.java @@ -677,8 +677,6 @@ public class SubmoduleOp { } private void logDebug(String msg, Object... args) { - if (logger.atFine().isEnabled()) { - logger.atFine().logVarargs(orm.getSubmissionId() + " " + msg, args); - } + logger.atFine().logVarargs(orm.getSubmissionId() + " " + msg, args); } } diff --git a/java/com/google/gerrit/server/submit/TestHelperOp.java b/java/com/google/gerrit/server/submit/TestHelperOp.java index e2a8b0987e..4f4d4095b2 100644 --- a/java/com/google/gerrit/server/submit/TestHelperOp.java +++ b/java/com/google/gerrit/server/submit/TestHelperOp.java @@ -50,8 +50,6 @@ class TestHelperOp implements BatchUpdateOp { } private void logDebug(String msg, Object... args) { - if (logger.atFine().isEnabled()) { - logger.atFine().logVarargs(submissionId + msg, args); - } + logger.atFine().logVarargs(submissionId + msg, args); } } diff --git a/java/com/google/gerrit/server/update/BatchUpdate.java b/java/com/google/gerrit/server/update/BatchUpdate.java index e665b7abc2..0e3465dbf9 100644 --- a/java/com/google/gerrit/server/update/BatchUpdate.java +++ b/java/com/google/gerrit/server/update/BatchUpdate.java @@ -385,7 +385,7 @@ public abstract class BatchUpdate implements AutoCloseable { } protected void logDebug(String msg, Throwable t) { - if (requestId != null && logger.atFine().isEnabled()) { + if (requestId != null) { logger.atFine().withCause(t).log(requestId + "%s", msg); } } @@ -394,7 +394,7 @@ public abstract class BatchUpdate implements AutoCloseable { // Only log if there is a requestId assigned, since those are the // expensive/complicated requests like MergeOp. Doing it every time would be // noisy. - if (requestId != null && logger.atFine().isEnabled()) { + if (requestId != null) { logger.atFine().logVarargs(requestId + msg, args); } } diff --git a/java/com/google/gerrit/server/update/ReviewDbBatchUpdate.java b/java/com/google/gerrit/server/update/ReviewDbBatchUpdate.java index 52838f9dd4..3c6f6fdf79 100644 --- a/java/com/google/gerrit/server/update/ReviewDbBatchUpdate.java +++ b/java/com/google/gerrit/server/update/ReviewDbBatchUpdate.java @@ -824,15 +824,11 @@ public class ReviewDbBatchUpdate extends BatchUpdate { } private void logDebug(String msg, Throwable t) { - if (logger.atFine().isEnabled()) { - ReviewDbBatchUpdate.this.logDebug("[" + taskId + "] " + msg, t); - } + ReviewDbBatchUpdate.this.logDebug("[" + taskId + "] " + msg, t); } private void logDebug(String msg, Object... args) { - if (logger.atFine().isEnabled()) { - ReviewDbBatchUpdate.this.logDebug("[" + taskId + "] " + msg, args); - } + ReviewDbBatchUpdate.this.logDebug("[" + taskId + "] " + msg, args); } }