From 07957409cb896ed01781b66a21590c18ea263721 Mon Sep 17 00:00:00 2001
From: zzz <zzz@mail.i2p>
Date: Sat, 14 Jan 2012 17:44:50 +0000
Subject: [PATCH]   * Stats:     - Cleanups     - Remove some locking     -
 Change some longs to ints to save space     - Remove static logs

---
 .../src/net/i2p/stat/BufferedStatLog.java     |  4 +-
 core/java/src/net/i2p/stat/Frequency.java     | 18 +-----
 .../src/net/i2p/stat/PersistenceHelper.java   | 41 +++++++++---
 core/java/src/net/i2p/stat/Rate.java          | 62 ++++++++++++++-----
 core/java/src/net/i2p/stat/RateStat.java      |  7 ++-
 core/java/src/net/i2p/stat/StatManager.java   |  9 +--
 6 files changed, 92 insertions(+), 49 deletions(-)

diff --git a/core/java/src/net/i2p/stat/BufferedStatLog.java b/core/java/src/net/i2p/stat/BufferedStatLog.java
index fc7b85c3f5..ae8a5b67aa 100644
--- a/core/java/src/net/i2p/stat/BufferedStatLog.java
+++ b/core/java/src/net/i2p/stat/BufferedStatLog.java
@@ -19,8 +19,8 @@ import net.i2p.util.Log;
  * be instantiated - see StatManager.
  */
 public class BufferedStatLog implements StatLog {
-    private I2PAppContext _context;
-    private Log _log;
+    private final I2PAppContext _context;
+    private final Log _log;
     private final StatEvent _events[];
     private int _eventNext;
     private int _lastWrite;
diff --git a/core/java/src/net/i2p/stat/Frequency.java b/core/java/src/net/i2p/stat/Frequency.java
index facde2fc00..9cf04ce2aa 100644
--- a/core/java/src/net/i2p/stat/Frequency.java
+++ b/core/java/src/net/i2p/stat/Frequency.java
@@ -18,7 +18,6 @@ public class Frequency {
     private long _lastEvent;
     private final long _start = now();
     private long _count;
-    private final Object _lock = this; // new Object(); // in case we want to do fancy sync later
 
     /** @param period ms */
     public Frequency(long period) {
@@ -37,9 +36,7 @@ public class Frequency {
      * @deprecated unused
      */
     public long getLastEvent() {
-        synchronized (_lock) {
             return _lastEvent;
-        }
     }
 
     /** 
@@ -48,9 +45,7 @@ public class Frequency {
      * @return milliseconds; returns period + 1 if no events in previous period
      */
     public double getAverageInterval() {
-        synchronized (_lock) {
             return _avgInterval;
-        }
     }
 
     /**
@@ -59,9 +54,7 @@ public class Frequency {
      * @deprecated unused
      */
     public double getMinAverageInterval() {
-        synchronized (_lock) {
             return _minAverageInterval;
-        }
     }
 
     /**
@@ -69,7 +62,7 @@ public class Frequency {
      * Use getStrictAverageInterval() for the real lifetime average.
      */
     public double getAverageEventsPerPeriod() {
-        synchronized (_lock) {
+        synchronized (this) {
             if (_avgInterval > 0) return _period / _avgInterval;
                 
             return 0;
@@ -81,7 +74,7 @@ public class Frequency {
      * Use getStrictAverageEventsPerPeriod() for the real lifetime average.
      */
     public double getMaxAverageEventsPerPeriod() {
-        synchronized (_lock) {
+        synchronized (this) {
             if (_minAverageInterval > 0 && _minAverageInterval <= _period) return _period / _minAverageInterval;
 
             return 0;
@@ -93,12 +86,9 @@ public class Frequency {
      * @return milliseconds; returns Double.MAX_VALUE if no events ever
      */
     public double getStrictAverageInterval() {
-        synchronized (_lock) {
             long duration = now() - _start;
             if ((duration <= 0) || (_count <= 0)) return Double.MAX_VALUE;
-           
             return duration / (double) _count;
-        }
     }
 
     /** using the strict average interval, how many events occur within an average period? */
@@ -110,9 +100,7 @@ public class Frequency {
 
     /** how many events have occurred within the lifetime of this stat? */
     public long getEventCount() {
-        synchronized (_lock) {
             return _count;
-        }
     }
 
     /** 
@@ -135,7 +123,7 @@ public class Frequency {
      * Recalculate, but only update the lastEvent if eventOccurred
      */
     private void recalculate(boolean eventOccurred) {
-        synchronized (_lock) {
+        synchronized (this) {
             // This calculates something of a rolling average interval.
             long now = now();
             long interval = now - _lastEvent;
diff --git a/core/java/src/net/i2p/stat/PersistenceHelper.java b/core/java/src/net/i2p/stat/PersistenceHelper.java
index 1d5c6a4663..f0994ff4f0 100644
--- a/core/java/src/net/i2p/stat/PersistenceHelper.java
+++ b/core/java/src/net/i2p/stat/PersistenceHelper.java
@@ -3,6 +3,7 @@ package net.i2p.stat;
 import java.util.Date;
 import java.util.Properties;
 
+import net.i2p.I2PAppContext;
 import net.i2p.data.DataHelper;
 import net.i2p.util.Log;
 
@@ -12,7 +13,6 @@ import net.i2p.util.Log;
  *  must be compatible.
  */
 class PersistenceHelper {
-    private final static Log _log = new Log(PersistenceHelper.class);
     private final static String NL = System.getProperty("line.separator");
 
     public final static void add(StringBuilder buf, String prefix, String name, String description, double value) {
@@ -33,37 +33,62 @@ class PersistenceHelper {
         add(buf, prefix, name, description + ' ' + when, value);
     }
 
+    /** @param value non-negative */
     public final static void add(StringBuilder buf, String prefix, String name, String description, long value) {
         buf.append("# ").append(prefix).append(name).append(NL);
         buf.append("# ").append(description).append(NL);
         buf.append(prefix).append(name).append('=').append(value).append(NL).append(NL);
     }
 
+    /**
+     *  @return non-negative, returns 0 on error
+     */
     public final static long getLong(Properties props, String prefix, String name) {
         String val = props.getProperty(prefix + name);
         if (val != null) {
             try {
-                return Long.parseLong(val);
+                long rv = Long.parseLong(val);
+                return rv >= 0 ? rv : 0;
             } catch (NumberFormatException nfe) {
-                _log.warn("Error formatting " + val + " into a long", nfe);
+                Log log = I2PAppContext.getGlobalContext().logManager().getLog(PersistenceHelper.class);
+                log.warn("Error formatting " + val, nfe);
             }
-        } else {
-            _log.warn("Key " + prefix + name + " does not exist");
         }
         return 0;
     }
 
+    /**
+     *  @return 0 on error
+     */
     public final static double getDouble(Properties props, String prefix, String name) {
         String val = props.getProperty(prefix + name);
         if (val != null) {
             try {
                 return Double.parseDouble(val);
             } catch (NumberFormatException nfe) {
-                _log.warn("Error formatting " + val + " into a double", nfe);
+                Log log = I2PAppContext.getGlobalContext().logManager().getLog(PersistenceHelper.class);
+                log.warn("Error formatting " + val, nfe);
             }
-        } else {
-            _log.warn("Key " + prefix + name + " does not exist");
         }
         return 0;
     }
+
+    /**
+     *  @return non-negative, returns 0 on error
+     *  @since 0.8.13
+     */
+    public final static int getInt(Properties props, String prefix, String name) {
+        String val = props.getProperty(prefix + name);
+        if (val != null) {
+            try {
+                int rv = Integer.parseInt(val);
+                return rv >= 0 ? rv : 0;
+            } catch (NumberFormatException nfe) {
+                Log log = I2PAppContext.getGlobalContext().logManager().getLog(PersistenceHelper.class);
+                log.warn("Error formatting " + val, nfe);
+            }
+        }
+        return 0;
+    }
+
 }
diff --git a/core/java/src/net/i2p/stat/Rate.java b/core/java/src/net/i2p/stat/Rate.java
index a234b48920..94d92a3ccf 100644
--- a/core/java/src/net/i2p/stat/Rate.java
+++ b/core/java/src/net/i2p/stat/Rate.java
@@ -16,13 +16,16 @@ import net.i2p.util.Log;
 public class Rate {
     //private final static Log _log = new Log(Rate.class);
     private volatile double _currentTotalValue;
-    private volatile long _currentEventCount;
+    // was long, save space
+    private volatile int _currentEventCount;
     private volatile long _currentTotalEventTime;
     private volatile double _lastTotalValue;
-    private volatile long _lastEventCount;
+    // was long, save space
+    private volatile int _lastEventCount;
     private volatile long _lastTotalEventTime;
     private volatile double _extremeTotalValue;
-    private volatile long _extremeEventCount;
+    // was long, save space
+    private volatile int _extremeEventCount;
     private volatile long _extremeTotalEventTime;
     private volatile double _lifetimeTotalValue;
     private volatile long _lifetimeEventCount;
@@ -32,7 +35,8 @@ public class Rate {
 
     private volatile long _lastCoalesceDate;
     private long _creationDate;
-    private long _period;
+    // was long, save space
+    private int _period;
 
     /** locked during coalesce and addData */
     // private final Object _lock = new Object();
@@ -121,15 +125,16 @@ public class Rate {
     /**
      * A rate with period shorter than Router.COALESCE_TIME = 50*1000 has to
      * be manually coalesced before values are fetched from it.
-     * @param period number of milliseconds in the period this rate deals with
-     * @throws IllegalArgumentException if the period is not greater than 0
+     * @param period number of milliseconds in the period this rate deals with, min 1, max Integer.MAX_VALUE
+     * @throws IllegalArgumentException if the period is invalid
      */
     public Rate(long period) throws IllegalArgumentException {
-        if (period <= 0) throw new IllegalArgumentException("The period must be strictly positive");
+        if (period <= 0 || period > Integer.MAX_VALUE)
+            throw new IllegalArgumentException();
 
         _creationDate = now();
         _lastCoalesceDate = _creationDate;
-        _period = period;
+        _period = (int) period;
     }
 
     /**
@@ -229,7 +234,7 @@ public class Rate {
             // how much were we off by?  (so that we can sample down the measured values)
             double periodFactor = measuredPeriod / (double)_period;
             _lastTotalValue = _currentTotalValue / periodFactor;
-            _lastEventCount = (long) (0.499999 + (_currentEventCount / periodFactor));
+            _lastEventCount = (int) (0.499999 + (_currentEventCount / periodFactor));
             _lastTotalEventTime = (long) (_currentTotalEventTime / periodFactor);
             _lastCoalesceDate = now;
             if (_currentEventCount == 0)
@@ -255,10 +260,15 @@ public class Rate {
     public void setSummaryListener(RateSummaryListener listener) { _summaryListener = listener; }
     public RateSummaryListener getSummaryListener() { return _summaryListener; }
     
-    /** what was the average value across the events in the last period? */
+    /**
+     * What was the average value across the events in the last period?
+     *
+     * Warning - unsynchronized, might glitch during coalesce, caller may prevent by synchronizing on this.
+     */
     public double getAverageValue() {
-        if ((_lastTotalValue != 0) && (_lastEventCount > 0))
-            return _lastTotalValue / _lastEventCount;
+        int lec = _lastEventCount;  // avoid race NPE
+        if ((_lastTotalValue != 0) && (lec > 0))
+            return _lastTotalValue / lec;
             
         return 0.0D;
     }
@@ -266,6 +276,8 @@ public class Rate {
     /**
      * During the extreme period (i.e. the period with the highest total value),
      * what was the average value?
+     *
+     * Warning - unsynchronized, might glitch during coalesce, caller may prevent by synchronizing on this.
      */
     public double getExtremeAverageValue() {
         if ((_extremeTotalValue != 0) && (_extremeEventCount > 0))
@@ -274,7 +286,11 @@ public class Rate {
         return 0.0D;
     }
 
-    /** what was the average value across the events since the stat was created? */
+    /**
+     * What was the average value across the events since the stat was created?
+     *
+     * Warning - unsynchronized, might glitch during coalesce, caller may prevent by synchronizing on this.
+     */
     public double getLifetimeAverageValue() {
         if ((_lifetimeTotalValue != 0) && (_lifetimeEventCount > 0))
             return _lifetimeTotalValue / _lifetimeEventCount;
@@ -306,6 +322,8 @@ public class Rate {
      * how much of the time was spent actually processing events
      * in proportion to how many events could have occurred if there were no intervals? 
      *
+     * Warning - unsynchronized, might glitch during coalesce, caller may prevent by synchronizing on this.
+     *
      * @return ratio, or 0 if the statistic doesn't use event times
      */
     public double getExtremeEventSaturation() {
@@ -321,6 +339,8 @@ public class Rate {
      * During the lifetime of this stat, how much of the time was spent actually processing events in proportion 
      * to how many events could have occurred if there were no intervals? 
      *
+     * Warning - unsynchronized, might glitch during coalesce, caller may prevent by synchronizing on this.
+     *
      * @return ratio, or 0 if event times aren't used
      */
     public double getLifetimeEventSaturation() {
@@ -345,6 +365,8 @@ public class Rate {
      * using the last period's rate, what is the total value that could have been sent 
      * if events were constant?
      *
+     * Warning - unsynchronized, might glitch during coalesce, caller may prevent by synchronizing on this.
+     *
      * @return max total value, or 0 if event times aren't used
      */
     public double getLastSaturationLimit() {
@@ -362,6 +384,8 @@ public class Rate {
      * what is the total value that could have been 
      * sent if events were constant?
      *
+     * Warning - unsynchronized, might glitch during coalesce, caller may prevent by synchronizing on this.
+     *
      * @return event total at saturation, or 0 if no event times are measured
      */
     public double getExtremeSaturationLimit() {
@@ -379,6 +403,8 @@ public class Rate {
      * What was the total value, compared to the total value in
      * the extreme period (i.e. the period with the highest total value),
      * Warning- returns ratio, not percentage (i.e. it is not multiplied by 100 here)
+     *
+     * Warning - unsynchronized, might glitch during coalesce, caller may prevent by synchronizing on this.
      */
     public double getPercentageOfExtremeValue() {
         if ((_lastTotalValue != 0) && (_extremeTotalValue != 0))
@@ -390,6 +416,8 @@ public class Rate {
     /**
      * How large was the last period's value as compared to the lifetime average value?
      * Warning- returns ratio, not percentage (i.e. it is not multiplied by 100 here)
+     *
+     * Warning - unsynchronized, might glitch during coalesce, caller may prevent by synchronizing on this.
      */
     public double getPercentageOfLifetimeValue() {
         if ((_lastTotalValue != 0) && (_lifetimeTotalValue != 0)) {
@@ -450,17 +478,17 @@ public class Rate {
      * @throws IllegalArgumentException if the data was formatted incorrectly
      */
     public void load(Properties props, String prefix, boolean treatAsCurrent) throws IllegalArgumentException {
-        _period = PersistenceHelper.getLong(props, prefix, ".period");
+        _period = PersistenceHelper.getInt(props, prefix, ".period");
         _creationDate = PersistenceHelper.getLong(props, prefix, ".creationDate");
         _lastCoalesceDate = PersistenceHelper.getLong(props, prefix, ".lastCoalesceDate");
         _currentTotalValue = PersistenceHelper.getDouble(props, prefix, ".currentTotalValue");
-        _currentEventCount = PersistenceHelper.getLong(props, prefix, ".currentEventCount");
+        _currentEventCount = PersistenceHelper.getInt(props, prefix, ".currentEventCount");
         _currentTotalEventTime = PersistenceHelper.getLong(props, prefix, ".currentTotalEventTime");
         _lastTotalValue = PersistenceHelper.getDouble(props, prefix, ".lastTotalValue");
-        _lastEventCount = PersistenceHelper.getLong(props, prefix, ".lastEventCount");
+        _lastEventCount = PersistenceHelper.getInt(props, prefix, ".lastEventCount");
         _lastTotalEventTime = PersistenceHelper.getLong(props, prefix, ".lastTotalEventTime");
         _extremeTotalValue = PersistenceHelper.getDouble(props, prefix, ".extremeTotalValue");
-        _extremeEventCount = PersistenceHelper.getLong(props, prefix, ".extremeEventCount");
+        _extremeEventCount = PersistenceHelper.getInt(props, prefix, ".extremeEventCount");
         _extremeTotalEventTime = PersistenceHelper.getLong(props, prefix, ".extremeTotalEventTime");
         _lifetimeTotalValue = PersistenceHelper.getDouble(props, prefix, ".lifetimeTotalValue");
         _lifetimeEventCount = PersistenceHelper.getLong(props, prefix, ".lifetimeEventCount");
diff --git a/core/java/src/net/i2p/stat/RateStat.java b/core/java/src/net/i2p/stat/RateStat.java
index 0920508457..0884de5870 100644
--- a/core/java/src/net/i2p/stat/RateStat.java
+++ b/core/java/src/net/i2p/stat/RateStat.java
@@ -7,12 +7,12 @@ import java.util.Map.Entry;
 import java.util.Properties;
 import java.util.concurrent.ConcurrentHashMap;
 
+import net.i2p.I2PAppContext;
 import net.i2p.data.DataHelper;
 import net.i2p.util.Log;
 
 /** coordinate a moving rate over various periods */
 public class RateStat {
-    private final static Log _log = new Log(RateStat.class);
     /** unique name of the statistic */
     private final String _statName;
     /** grouping under which the stat is kept */
@@ -223,8 +223,9 @@ public class RateStat {
                 Rate rate = new Rate(period);
                 rate.setRateStat(this);
                 _rates.put(rate.getPeriod(), rate);
-                if (_log.shouldLog(Log.WARN))
-                    _log.warn("Rate for " + prefix + " is corrupt, reinitializing that period");
+                Log log = I2PAppContext.getGlobalContext().logManager().getLog(RateStat.class);
+                if (log.shouldLog(Log.WARN))
+                    log.warn("Rate for " + prefix + " is corrupt, reinitializing that period");
             }
         }
     }
diff --git a/core/java/src/net/i2p/stat/StatManager.java b/core/java/src/net/i2p/stat/StatManager.java
index ba50620ae9..9b00145a11 100644
--- a/core/java/src/net/i2p/stat/StatManager.java
+++ b/core/java/src/net/i2p/stat/StatManager.java
@@ -6,6 +6,7 @@ import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Set;
+import java.util.SortedSet;
 import java.util.TreeMap;
 import java.util.TreeSet;
 import java.util.concurrent.ConcurrentHashMap;
@@ -213,18 +214,18 @@ public class StatManager {
     }
 
     /** Group name (String) to a Set of stat names, ordered alphabetically */
-    public Map getStatsByGroup() {
-        Map groups = new TreeMap(Collator.getInstance());
+    public Map<String, SortedSet<String>> getStatsByGroup() {
+        Map<String, SortedSet<String>> groups = new TreeMap(Collator.getInstance());
         for (Iterator<FrequencyStat> iter = _frequencyStats.values().iterator(); iter.hasNext();) {
             FrequencyStat stat = iter.next();
             if (!groups.containsKey(stat.getGroupName())) groups.put(stat.getGroupName(), new TreeSet());
-            Set names = (Set) groups.get(stat.getGroupName());
+            Set<String> names = groups.get(stat.getGroupName());
             names.add(stat.getName());
         }
         for (Iterator<RateStat> iter = _rateStats.values().iterator(); iter.hasNext();) {
             RateStat stat = iter.next();
             if (!groups.containsKey(stat.getGroupName())) groups.put(stat.getGroupName(), new TreeSet());
-            Set names = (Set) groups.get(stat.getGroupName());
+            Set<String> names = groups.get(stat.getGroupName());
             names.add(stat.getName());
         }
         return groups;
-- 
GitLab