----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36186/#review90517 -----------------------------------------------------------
core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java (line 193) <https://reviews.apache.org/r/36186/#comment143635> I added this OpTimer thing a long time ago. One of the reasons I added was to collapse lines of code. Like this line replaces 1 line with 4. The changes in the patch also avoid object creation. I was trying to think of a way to avoid object creation and collapse lines of code. Could have static util method like the following. I am ok w/ not doing this (its not that much repetition), its just something I thought of when looking at the code. ```java public static EventTimer startTimer(Logger log, String msg, Object ... args) { if (log.isTraceEnabled()) { log.trace(msg, args); return new EventTimer().start(); } return null; } //this method avoids allocating array for single argument.. would probably want method w/ two ares like slf4j has public static EventTimer startTimer(Logger log, String msg, Object arg) { if (log.isTraceEnabled()) { log.trace(msg, arg); return new EventTimer().start(); } return null; } ``` - kturner On July 6, 2015, 6:07 p.m., Ed Coleman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36186/ > ----------------------------------------------------------- > > (Updated July 6, 2015, 6:07 p.m.) > > > Review request for accumulo. > > > Bugs: ACCUMULO-3840 > https://issues.apache.org/jira/browse/ACCUMULO-3840 > > > Repository: accumulo > > > Description > ------- > > As part of the removal of direct log4j dependecies - replaced OpTimer > > > Diffs > ----- > > core/src/main/java/org/apache/accumulo/core/client/ZooKeeperInstance.java > c5cb482 > > core/src/main/java/org/apache/accumulo/core/client/impl/NamespaceOperationsImpl.java > b087c73 > > core/src/main/java/org/apache/accumulo/core/client/impl/RootTabletLocator.java > 3a0c0d7 > > core/src/main/java/org/apache/accumulo/core/client/impl/TableOperationsImpl.java > 6f9ea29 > > core/src/main/java/org/apache/accumulo/core/client/impl/TabletLocatorImpl.java > c28320d > > core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchReaderIterator.java > f263581 > core/src/main/java/org/apache/accumulo/core/client/impl/ThriftScanner.java > 5437cc4 > > core/src/main/java/org/apache/accumulo/core/metadata/MetadataLocationObtainer.java > 0d294b8 > core/src/main/java/org/apache/accumulo/core/util/EventTimer.java > PRE-CREATION > core/src/main/java/org/apache/accumulo/core/util/OpTimer.java 564a824 > core/src/test/java/org/apache/accumulo/core/util/EventTimerTest.java > PRE-CREATION > > server/base/src/main/java/org/apache/accumulo/server/client/HdfsZooInstance.java > 0d7aaf1 > test/src/main/java/org/apache/accumulo/test/NativeMapStressTest.java > 72831d8 > > Diff: https://reviews.apache.org/r/36186/diff/ > > > Testing > ------- > > passes > > mvn verify -DskipITs > mvn package -P assemble > > > Thanks, > > Ed Coleman > >
