nfsantos commented on code in PR #813:
URL: https://github.com/apache/jackrabbit-oak/pull/813#discussion_r1064505114
##########
oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/ItemImpl.java:
##########
@@ -17,6 +17,7 @@
package org.apache.jackrabbit.oak.jcr.session;
import static com.google.common.collect.Lists.newArrayListWithCapacity;
+import static java.lang.String.format;
Review Comment:
Nitpick, personal style preference: I was confused reading the code,
wondering if the method call format() was part of any 3rd party library or some
util class we have made. It took me some time until realizing that it was a
static import. My personal preference is to avoid static imports unless it is
idiomatic (like Unit tests assert methods), as I think they can make the code a
bit less readable.
##########
oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/delegate/SessionDelegate.java:
##########
@@ -78,6 +79,23 @@ public class SessionDelegate {
static final Logger readOperationLogger =
LoggerFactory.getLogger("org.apache.jackrabbit.oak.jcr.operations.reads");
static final Logger writeOperationLogger =
LoggerFactory.getLogger("org.apache.jackrabbit.oak.jcr.operations.writes");
+ // the bitmask used for trace level logging
+ // we use a bitmask instead of a counter to avoid the slow modulo
operation:
+ //
https://stackoverflow.com/questions/27977834/why-is-modulus-operator-slow
+ // so we use "if ((counter & LOG_TRACE_BIT_MASK) == 0) log(...)"
+ // instead of the slower "if ((counter % LOG_TRACE) == 0) log(...)"
+ // that means the values need to be some power of two, minus one
+ // log every 128th call by default
+ private static final long LOG_TRACE_BIT_MASK = Long.getLong(
+ "org.apache.jackrabbit.oak.jcr.operations.bitMask",
+ 128L - 1);
+ // log a stack trace every ~1 million calls by default
+ private static final long LOG_TRACE_STACK_BIT_MASK = Long.getLong(
+ "org.apache.jackrabbit.oak.jcr.operations.stack.bitMask",
+ 1024L * 1024 - 1);
+ // the counter used for logging
+ private static final AtomicLong LOG_COUNTER = new AtomicLong();
Review Comment:
What will happen if the counter overflows? I guess the bit shift operations
will continue giving the expected results, but just want to be sure you
considered this case. Not sure how long it will take to overflow a long, might
be one of those age-of-universe situations, but better be sure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
