From b4e4a53db6ec9bcb794e0bcb0933f2da71e9a873 Mon Sep 17 00:00:00 2001 From: Joel Richard Date: Mon, 7 Sep 2015 12:37:04 +0200 Subject: [PATCH] OAK-3361 - Reduce PerfLogger isDebugEnabled overhead * Skip isDebugEnabled() if start is negative --- .../org/apache/jackrabbit/oak/util/PerfLogger.java | 41 ++++++-------- .../apache/jackrabbit/oak/util/PerfLoggerTest.java | 65 ++++++++++++++++++++++ 2 files changed, 82 insertions(+), 24 deletions(-) create mode 100644 oak-core/src/test/java/org/apache/jackrabbit/oak/util/PerfLoggerTest.java diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/util/PerfLogger.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/util/PerfLogger.java index a0cd1d8..ca3d2f3 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/util/PerfLogger.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/util/PerfLogger.java @@ -109,7 +109,7 @@ public final class PerfLogger { */ public final void end(long start, long logAtDebugIfSlowerThanMs, String logMessagePrefix, Object arg1) { - if (!delegate.isDebugEnabled()) { + if (start < 0) { return; } end(start, logAtDebugIfSlowerThanMs, logMessagePrefix, @@ -135,7 +135,7 @@ public final class PerfLogger { */ public final void end(long start, long logAtDebugIfSlowerThanMs, String logMessagePrefix, Object arg1, Object arg2) { - if (!delegate.isDebugEnabled()) { + if (start < 0) { return; } end(start, logAtDebugIfSlowerThanMs, logMessagePrefix, new Object[] { @@ -157,32 +157,25 @@ public final class PerfLogger { */ public void end(long start, long logAtDebugIfSlowerThanMs, String logMessagePrefix, Object... arguments) { - if (!delegate.isDebugEnabled()) { - // if log level is not at least DEBUG, then return fast, no-op + if (start < 0) { + // if start is negative, we assume that start() returned -1 because the log level is above DEBUG return; } - if (start == -1) { - // start was never set - // -> then log at trace as we have no diff available - delegate.trace(logMessagePrefix + " [start not set]", + final long diff = System.currentTimeMillis() - start; + if (delegate.isTraceEnabled()) { + // if log level is TRACE, then always log - and do that on TRACE + // then: + delegate.trace(logMessagePrefix + " [took " + diff + "ms]", + (Object[]) arguments); + } else if ((logAtDebugIfSlowerThanMs < 0) + || (diff > logAtDebugIfSlowerThanMs)) { + // otherwise (log level is DEBUG) only log if + // logDebugIfSlowerThanMs is set to -1 (or negative) + // OR the measured diff is larger than logDebugIfSlowerThanMs - + // and then do that on DEBUG: + delegate.debug(logMessagePrefix + " [took " + diff + "ms]", (Object[]) arguments); - } else { - final long diff = System.currentTimeMillis() - start; - if (delegate.isTraceEnabled()) { - // if log level is TRACE, then always log - and do that on TRACE - // then: - delegate.trace(logMessagePrefix + " [took " + diff + "ms]", - (Object[]) arguments); - } else if ((logAtDebugIfSlowerThanMs < 0) - || (diff > logAtDebugIfSlowerThanMs)) { - // otherwise (log level is DEBUG) only log if - // logDebugIfSlowerThanMs is set to -1 (or negative) - // OR the measured diff is larger than logDebugIfSlowerThanMs - - // and then do that on DEBUG: - delegate.debug(logMessagePrefix + " [took " + diff + "ms]", - (Object[]) arguments); - } } } diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/util/PerfLoggerTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/util/PerfLoggerTest.java new file mode 100644 index 0000000..bf8d616 --- /dev/null +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/util/PerfLoggerTest.java @@ -0,0 +1,65 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.jackrabbit.oak.util; + +import org.junit.Test; +import org.mockito.Mockito; +import org.slf4j.Logger; + +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyString; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +public class PerfLoggerTest { + + @Test + public void testEndDebug() { + Logger logger = Mockito.mock(Logger.class); + when(logger.isTraceEnabled()).thenReturn(false); + when(logger.isDebugEnabled()).thenReturn(true); + + PerfLogger perfLogger = new PerfLogger(logger); + long start = perfLogger.start(); + perfLogger.end(start, -1, "message", "argument"); + + verify(logger, atLeastOnce()).isTraceEnabled(); + verify(logger, atLeastOnce()).isDebugEnabled(); + verify(logger, times(1)).debug(anyString(), any(Object[].class)); + verifyNoMoreInteractions(logger); + } + + @Test + public void testEndSkipsIsDebugEnabled() { + Logger logger = Mockito.mock(Logger.class); + when(logger.isTraceEnabled()).thenReturn(false); + when(logger.isDebugEnabled()).thenReturn(false); + + PerfLogger perfLogger = new PerfLogger(logger); + long start = perfLogger.start(); + perfLogger.end(start, -1, "message", "argument"); + + verify(logger, never()).isTraceEnabled(); + verify(logger, times(1)).isDebugEnabled(); + verifyNoMoreInteractions(logger); + } + +} -- 2.3.6