opendev/gerrit
Code Issues Proposed changes

Remove unneeded checks from logDebug methods to check if fine logging is enabled

Browse Source 
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 <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2018-06-08 10:36:13 +02:00
parent 653616ade8
commit bf734b9d77
8 changed files with 10 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
java/com/google/gerrit/server/git/receive/ReceiveCommits.java
View File

@@ -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) {

4
java/com/google/gerrit/server/submit/GitModules.java
View File

@@ -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);
}
}

4
java/com/google/gerrit/server/submit/MergeOp.java
View File

@@ -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) {

4
java/com/google/gerrit/server/submit/SubmitStrategyOp.java
View File

@@ -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) {

4
java/com/google/gerrit/server/submit/SubmoduleOp.java
View File

@@ -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);
}
}

4
java/com/google/gerrit/server/submit/TestHelperOp.java
View File

@@ -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);
}
}

4
java/com/google/gerrit/server/update/BatchUpdate.java
View File

@@ -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);
}
}

8
java/com/google/gerrit/server/update/ReviewDbBatchUpdate.java
View File

@@ -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);
}
}