From 03e52fab2b71ecbd7e7ad6b625411f092ae7d860 Mon Sep 17 00:00:00 2001
From: nitsanw <nitsanw@yahoo.com>
Date: Mon, 27 Jul 2015 14:26:02 +0200
Subject: [PATCH] Refactor OneMeasurement: Make returncodes private final, make
 _name final, refactor repeated export code from subclasses.

---
 .../ycsb/measurements/OneMeasurement.java     |  15 +-
 .../OneMeasurementHdrHistogram.java           | 170 +++++++++---------
 .../measurements/OneMeasurementHistogram.java |   4 +-
 .../OneMeasurementTimeSeries.java             |  16 +-
 4 files changed, 99 insertions(+), 106 deletions(-)

diff --git a/core/src/main/java/com/yahoo/ycsb/measurements/OneMeasurement.java b/core/src/main/java/com/yahoo/ycsb/measurements/OneMeasurement.java
index 45c6a561..3c613fe3 100644
--- a/core/src/main/java/com/yahoo/ycsb/measurements/OneMeasurement.java
+++ b/core/src/main/java/com/yahoo/ycsb/measurements/OneMeasurement.java
@@ -29,8 +29,8 @@ import com.yahoo.ycsb.measurements.exporter.MeasurementsExporter;
  */
 public abstract class OneMeasurement {
 
-  String _name;
-  final ConcurrentHashMap<Integer, AtomicInteger> returncodes;
+  private final String _name;
+  private  final ConcurrentHashMap<Integer, AtomicInteger> _returncodes;
 
   public String getName() {
     return _name;
@@ -41,7 +41,7 @@ public abstract class OneMeasurement {
    */
   public OneMeasurement(String _name) {
     this._name = _name;
-    this.returncodes = new ConcurrentHashMap<Integer, AtomicInteger>();
+    this._returncodes = new ConcurrentHashMap<Integer, AtomicInteger>();
   }
 
   public abstract void measure(int latency);
@@ -53,10 +53,10 @@ public abstract class OneMeasurement {
    */
   public void reportReturnCode(int code) {
     Integer Icode = code;
-    AtomicInteger counter = returncodes.get(Icode);
+    AtomicInteger counter = _returncodes.get(Icode);
 
     if (counter == null) {
-      AtomicInteger other = returncodes.putIfAbsent(Icode, counter = new AtomicInteger());
+      AtomicInteger other = _returncodes.putIfAbsent(Icode, counter = new AtomicInteger());
       if (other != null) {
         counter = other;
       }
@@ -73,4 +73,9 @@ public abstract class OneMeasurement {
    */
   public abstract void exportMeasurements(MeasurementsExporter exporter) throws IOException;
 
+  protected final void exportReturnCodes(MeasurementsExporter exporter) throws IOException {
+    for (Map.Entry<Integer, AtomicInteger> entry : _returncodes.entrySet()) {
+      exporter.write(getName(), "Return=" + entry.getKey(), entry.getValue().get());
+    }
+  }
 }
diff --git a/core/src/main/java/com/yahoo/ycsb/measurements/OneMeasurementHdrHistogram.java b/core/src/main/java/com/yahoo/ycsb/measurements/OneMeasurementHdrHistogram.java
index 07316324..377cb105 100644
--- a/core/src/main/java/com/yahoo/ycsb/measurements/OneMeasurementHdrHistogram.java
+++ b/core/src/main/java/com/yahoo/ycsb/measurements/OneMeasurementHdrHistogram.java
@@ -34,63 +34,63 @@ import org.HdrHistogram.Recorder;
 import com.yahoo.ycsb.measurements.exporter.MeasurementsExporter;
 
 /**
- * Take measurements and maintain a HdrHistogram of a given metric, such as
- * READ LATENCY.
+ * Take measurements and maintain a HdrHistogram of a given metric, such as READ
+ * LATENCY.
  *
  * @author nitsanw
  *
  */
 public class OneMeasurementHdrHistogram extends OneMeasurement {
 
-  // we need one log per measurement histogram
-  final PrintStream log;
-  final HistogramLogWriter histogramLogWriter;
-
-  final Recorder histogram = new Recorder(3);
-
-  Histogram totalHistogram;
-
-  public OneMeasurementHdrHistogram(String name, Properties props) {
-    super(name);
-    boolean shouldLog = Boolean.parseBoolean(props.getProperty("hdrhistogram.fileoutput", "false"));
-    if (!shouldLog) {
-      log = null;
-      histogramLogWriter = null;
-      return;
-    }
-    try {
-      final String hdrOutputFilename = props.getProperty("hdrhistogram.output.path", "") +name+".hdr";
-      log = new PrintStream(new FileOutputStream(hdrOutputFilename), false);
-    } catch (FileNotFoundException e) {
-      throw new RuntimeException("Failed to open hdr histogram output file", e);
-    }
-    histogramLogWriter = new HistogramLogWriter(log);
-    histogramLogWriter.outputComment("[Logging for: " + name + "]");
-    histogramLogWriter.outputLogFormatVersion();
-    histogramLogWriter.outputStartTime(System.currentTimeMillis());
-    histogramLogWriter.outputLegend();
-  }
-
-  /**
-    * It appears latency is reported in micros.
-    * Using {@link Recorder} to support concurrent updates to histogram.
-    *
-    * @see com.yahoo.ycsb.OneMeasurement#measure(int)
-    */
-  public void measure(int latencyInMicros) {
-    histogram.recordValue(latencyInMicros);
-  }
-
-  /**
-    * This is called from a main thread, on orderly termination.
-    *
-    * @see com.yahoo.ycsb.measurements.OneMeasurement#exportMeasurements(com.yahoo.ycsb.measurements.exporter.MeasurementsExporter)
-    */
+	// we need one log per measurement histogram
+	final PrintStream log;
+	final HistogramLogWriter histogramLogWriter;
+
+	final Recorder histogram = new Recorder(3);
+
+	Histogram totalHistogram;
+
+	public OneMeasurementHdrHistogram(String name, Properties props) {
+		super(name);
+		boolean shouldLog = Boolean.parseBoolean(props.getProperty("hdrhistogram.fileoutput", "false"));
+		if (!shouldLog) {
+			log = null;
+			histogramLogWriter = null;
+			return;
+		}
+		try {
+			final String hdrOutputFilename = props.getProperty("hdrhistogram.output.path", "") + name + ".hdr";
+			log = new PrintStream(new FileOutputStream(hdrOutputFilename), false);
+		} catch (FileNotFoundException e) {
+			throw new RuntimeException("Failed to open hdr histogram output file", e);
+		}
+		histogramLogWriter = new HistogramLogWriter(log);
+		histogramLogWriter.outputComment("[Logging for: " + name + "]");
+		histogramLogWriter.outputLogFormatVersion();
+		histogramLogWriter.outputStartTime(System.currentTimeMillis());
+		histogramLogWriter.outputLegend();
+	}
+
+	/**
+	 * It appears latency is reported in micros. Using {@link Recorder} to
+	 * support concurrent updates to histogram.
+	 *
+	 * @see com.yahoo.ycsb.OneMeasurement#measure(int)
+	 */
+	public void measure(int latencyInMicros) {
+		histogram.recordValue(latencyInMicros);
+	}
+
+	/**
+	 * This is called from a main thread, on orderly termination.
+	 *
+	 * @see com.yahoo.ycsb.measurements.OneMeasurement#exportMeasurements(com.yahoo.ycsb.measurements.exporter.MeasurementsExporter)
+	 */
   @Override
   public void exportMeasurements(MeasurementsExporter exporter) throws IOException {
     // accumulate the last interval which was not caught by status thread
     Histogram intervalHistogram = getIntervalHistogramAndAccumulate();
-    if(histogramLogWriter != null) {
+    if (histogramLogWriter != null) {
       histogramLogWriter.outputIntervalHistogram(intervalHistogram);
       // we can close now
       log.close();
@@ -101,47 +101,43 @@ public class OneMeasurementHdrHistogram extends OneMeasurement {
     exporter.write(getName(), "MaxLatency(us)", totalHistogram.getMaxValue());
     exporter.write(getName(), "95thPercentileLatency(ms)", totalHistogram.getValueAtPercentile(90)/1000);
     exporter.write(getName(), "99thPercentileLatency(ms)", totalHistogram.getValueAtPercentile(99)/1000);
-
-    for (Map.Entry<Integer, AtomicInteger> entry : returncodes.entrySet()) {
-      exporter.write(getName(), "Return=" + entry.getKey(), entry.getValue().get());
-    }
+    
+    exportReturnCodes(exporter);
   }
 
-  /**
-    * This is called periodically from the StatusThread. There's a single StatusThread per Client process.
-    * We optionally serialize the interval to log on this opportunity.
-    * @see com.yahoo.ycsb.measurements.OneMeasurement#getSummary()
-    */
-  @Override
-  public String getSummary() {
-    Histogram intervalHistogram = getIntervalHistogramAndAccumulate();
-    // we use the summary interval as the histogram file interval.
-    if(histogramLogWriter != null) {
-      histogramLogWriter.outputIntervalHistogram(intervalHistogram);
-    }
-
-    DecimalFormat d = new DecimalFormat("#.##");
-    return "[" + getName() +
-            ": Count=" + intervalHistogram.getTotalCount() +
-            ", Max=" + intervalHistogram.getMaxValue() +
-            ", Min=" + intervalHistogram.getMinValue() +
-            ", Avg=" + d.format(intervalHistogram.getMean()) +
-            ", 90=" + d.format(intervalHistogram.getValueAtPercentile(90)) +
-            ", 99=" + d.format(intervalHistogram.getValueAtPercentile(99)) +
-            ", 99.9=" + d.format(intervalHistogram.getValueAtPercentile(99.9)) +
-            ", 99.99=" + d.format(intervalHistogram.getValueAtPercentile(99.99)) +"]";
-  }
-
-  private Histogram getIntervalHistogramAndAccumulate() {
-      Histogram intervalHistogram = histogram.getIntervalHistogram();
-      // add this to the total time histogram.
-      if (totalHistogram == null) {
-        totalHistogram = intervalHistogram;
-      }
-      else {
-        totalHistogram.add(intervalHistogram);
-      }
-      return intervalHistogram;
-  }
+	/**
+	 * This is called periodically from the StatusThread. There's a single
+	 * StatusThread per Client process. We optionally serialize the interval to
+	 * log on this opportunity.
+	 * 
+	 * @see com.yahoo.ycsb.measurements.OneMeasurement#getSummary()
+	 */
+	@Override
+	public String getSummary() {
+		Histogram intervalHistogram = getIntervalHistogramAndAccumulate();
+		// we use the summary interval as the histogram file interval.
+		if (histogramLogWriter != null) {
+			histogramLogWriter.outputIntervalHistogram(intervalHistogram);
+		}
+
+		DecimalFormat d = new DecimalFormat("#.##");
+		return "[" + getName() + ": Count=" + intervalHistogram.getTotalCount() + ", Max="
+				+ intervalHistogram.getMaxValue() + ", Min=" + intervalHistogram.getMinValue() + ", Avg="
+				+ d.format(intervalHistogram.getMean()) + ", 90=" + d.format(intervalHistogram.getValueAtPercentile(90))
+				+ ", 99=" + d.format(intervalHistogram.getValueAtPercentile(99)) + ", 99.9="
+				+ d.format(intervalHistogram.getValueAtPercentile(99.9)) + ", 99.99="
+				+ d.format(intervalHistogram.getValueAtPercentile(99.99)) + "]";
+	}
+
+	private Histogram getIntervalHistogramAndAccumulate() {
+		Histogram intervalHistogram = histogram.getIntervalHistogram();
+		// add this to the total time histogram.
+		if (totalHistogram == null) {
+			totalHistogram = intervalHistogram;
+		} else {
+			totalHistogram.add(intervalHistogram);
+		}
+		return intervalHistogram;
+	}
 
 }
diff --git a/core/src/main/java/com/yahoo/ycsb/measurements/OneMeasurementHistogram.java b/core/src/main/java/com/yahoo/ycsb/measurements/OneMeasurementHistogram.java
index d8305b30..90299206 100644
--- a/core/src/main/java/com/yahoo/ycsb/measurements/OneMeasurementHistogram.java
+++ b/core/src/main/java/com/yahoo/ycsb/measurements/OneMeasurementHistogram.java
@@ -119,9 +119,7 @@ public class OneMeasurementHistogram extends OneMeasurement
       }
     }
 
-    for (Map.Entry<Integer, AtomicInteger> entry : returncodes.entrySet()) {
-      exporter.write(getName(), "Return=" + entry.getKey(), entry.getValue().get());
-    }
+    exportReturnCodes(exporter);
 
     for (int i=0; i<_buckets; i++)
     {
diff --git a/core/src/main/java/com/yahoo/ycsb/measurements/OneMeasurementTimeSeries.java b/core/src/main/java/com/yahoo/ycsb/measurements/OneMeasurementTimeSeries.java
index d4871660..8794f896 100644
--- a/core/src/main/java/com/yahoo/ycsb/measurements/OneMeasurementTimeSeries.java
+++ b/core/src/main/java/com/yahoo/ycsb/measurements/OneMeasurementTimeSeries.java
@@ -125,24 +125,18 @@ public class OneMeasurementTimeSeries extends OneMeasurement
 
 
   @Override
-  public void exportMeasurements(MeasurementsExporter exporter) throws IOException
-  {
+  public void exportMeasurements(MeasurementsExporter exporter) throws IOException {
     checkEndOfUnit(true);
 
     exporter.write(getName(), "Operations", operations);
-    exporter.write(getName(), "AverageLatency(us)", (((double)totallatency)/((double)operations)));
+    exporter.write(getName(), "AverageLatency(us)", (((double) totallatency) / ((double) operations)));
     exporter.write(getName(), "MinLatency(us)", min);
     exporter.write(getName(), "MaxLatency(us)", max);
 
-    //TODO: 95th and 99th percentile latency
-
+    // TODO: 95th and 99th percentile latency
 
-    for (Map.Entry<Integer, AtomicInteger> entry : returncodes.entrySet()) {
-      exporter.write(getName(), "Return=" + entry.getKey(), entry.getValue().get());
-    }
-
-    for (SeriesUnit unit : _measurements)
-    {
+    exportReturnCodes(exporter);
+    for (SeriesUnit unit : _measurements) {
       exporter.write(getName(), Long.toString(unit.time), unit.average);
     }
   }
-- 
GitLab