From ce5ce12e3bd6a8457abfd822c2651087d9d50300 Mon Sep 17 00:00:00 2001 From: zzz Date: Wed, 16 Mar 2011 16:11:54 +0000 Subject: [PATCH] * Naming services: - Refactor caching - Logging, caching, shutdown cleanup and fixes --- .../client/naming/BlockfileNamingService.java | 15 +-- .../i2p/client/naming/DummyNamingService.java | 99 ++++++------------- .../client/naming/EepGetNamingService.java | 1 - .../i2p/client/naming/ExecNamingService.java | 1 - .../i2p/client/naming/MetaNamingService.java | 6 +- .../net/i2p/client/naming/NamingService.java | 6 +- .../naming/SingleFileNamingService.java | 28 ++++-- 7 files changed, 71 insertions(+), 85 deletions(-) diff --git a/core/java/src/net/i2p/client/naming/BlockfileNamingService.java b/core/java/src/net/i2p/client/naming/BlockfileNamingService.java index 4fa0477e2..931861ec6 100644 --- a/core/java/src/net/i2p/client/naming/BlockfileNamingService.java +++ b/core/java/src/net/i2p/client/naming/BlockfileNamingService.java @@ -71,7 +71,6 @@ import net.metanotion.util.skiplist.SkipList; */ public class BlockfileNamingService extends DummyNamingService { - private final Log _log; private final BlockFile _bf; private final RandomAccessFile _raf; private final List _lists; @@ -104,7 +103,6 @@ public class BlockfileNamingService extends DummyNamingService { */ public BlockfileNamingService(I2PAppContext context) { super(context); - _log = context.logManager().getLog(BlockfileNamingService.class); _lists = new ArrayList(); BlockFile bf = null; RandomAccessFile raf = null; @@ -199,10 +197,11 @@ public class BlockfileNamingService extends DummyNamingService { if (in != null) try { in.close(); } catch (IOException ioe) {} } total += count; - _log.logAlways(Log.INFO, "Added " + count + " hosts from " + file); + _log.logAlways(Log.INFO, "Migrating " + count + " hosts from " + file + " to new hosts database"); _lists.add(hostsfile); } - _log.error("DB init took " + DataHelper.formatDuration(_context.clock().now() - start)); + if (_log.shouldLog(Log.INFO)) + _log.info("DB init took " + DataHelper.formatDuration(_context.clock().now() - start)); if (total <= 0) _log.error("Warning - initialized database with zero entries"); return rv; @@ -239,8 +238,9 @@ public class BlockfileNamingService extends DummyNamingService { createdOn = Long.parseLong(created); } catch (NumberFormatException nfe) {} } - _log.error("Found database version " + version + " created " + (new Date(createdOn)).toString() + - " containing lists: " + list); + if (_log.shouldLog(Log.INFO)) + _log.info("Found database version " + version + " created " + (new Date(createdOn)).toString() + + " containing lists: " + list); List skiplists = getFilenames(list); if (skiplists.isEmpty()) @@ -422,6 +422,8 @@ public class BlockfileNamingService extends DummyNamingService { if (changed && checkExisting) return false; addEntry(sl, key, d, props); + if (changed) + removeCache(hostname); for (NamingServiceListener nsl : _listeners) { if (changed) nsl.entryChanged(this, hostname, d, options); @@ -460,6 +462,7 @@ public class BlockfileNamingService extends DummyNamingService { return false; boolean rv = removeEntry(sl, key) != null; if (rv) { + removeCache(hostname); for (NamingServiceListener nsl : _listeners) { nsl.entryRemoved(this, key); } diff --git a/core/java/src/net/i2p/client/naming/DummyNamingService.java b/core/java/src/net/i2p/client/naming/DummyNamingService.java index 1f8d96799..ea79cd04c 100644 --- a/core/java/src/net/i2p/client/naming/DummyNamingService.java +++ b/core/java/src/net/i2p/client/naming/DummyNamingService.java @@ -7,9 +7,7 @@ */ package net.i2p.client.naming; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; +import java.util.LinkedHashMap; import java.util.Map; import java.util.Properties; @@ -20,13 +18,19 @@ import net.i2p.data.Destination; * A Dummy naming service that can only handle base64 and b32 destinations. */ class DummyNamingService extends NamingService { - private final Map _cache; private static final int BASE32_HASH_LENGTH = 52; // 1 + Hash.HASH_LENGTH * 8 / 5 public final static String PROP_B32 = "i2p.naming.hostsTxt.useB32"; - protected static final int CACHE_MAX_SIZE = 16; + protected static final int CACHE_MAX_SIZE = 32; public static final int DEST_SIZE = 516; // Std. Base64 length (no certificate) + /** + * The LRU cache, with no expiration time. + * Classes should take care to call removeCache() for any entries that + * are invalidated. + */ + private static final Map _cache = new LHM(CACHE_MAX_SIZE); + /** * The naming service should only be constructed and accessed through the * application context. This constructor should only be used by the @@ -35,7 +39,6 @@ class DummyNamingService extends NamingService { */ protected DummyNamingService(I2PAppContext context) { super(context); - _cache = new HashMap(CACHE_MAX_SIZE); } @Override @@ -66,88 +69,48 @@ class DummyNamingService extends NamingService { } /** - * Provide basic caching for the service - * The service may override the age and/or size limit + * Provide basic static caching for all services */ - /** Don't know why a dest would ever change but keep this short anyway */ - protected static final long CACHE_MAX_AGE = 7*60*1000; - - private class CacheEntry { - public Destination dest; - public long exp; - public CacheEntry(Destination d) { - dest = d; - exp = _context.clock().now() + CACHE_MAX_AGE; - } - public boolean isExpired() { - return exp < _context.clock().now(); - } - } - - /** - * Clean up when full. - * Don't bother removing old entries unless full. - * Caller must synchronize on _cache. - */ - private void cacheClean() { - if (_cache.size() < CACHE_MAX_SIZE) - return; - boolean full = true; - String oldestKey = null; - long oldestExp = Long.MAX_VALUE; - List deleteList = new ArrayList(CACHE_MAX_SIZE); - for (Map.Entry entry : _cache.entrySet()) { - CacheEntry ce = entry.getValue(); - if (ce.isExpired()) { - deleteList.add(entry.getKey()); - full = false; - continue; - } - if (oldestKey == null || ce.exp < oldestExp) { - oldestKey = entry.getKey(); - oldestExp = ce.exp; - } - } - if (full && oldestKey != null) - deleteList.add(oldestKey); - for (String s : deleteList) { - _cache.remove(s); - } - } - - protected void putCache(String s, Destination d) { + protected static void putCache(String s, Destination d) { if (d == null) return; synchronized (_cache) { - _cache.put(s, new CacheEntry(d)); - cacheClean(); + _cache.put(s, d); } } - protected Destination getCache(String s) { + /** @return cached dest or null */ + protected static Destination getCache(String s) { synchronized (_cache) { - CacheEntry ce = _cache.get(s); - if (ce == null) - return null; - if (ce.isExpired()) { - _cache.remove(s); - return null; - } - return ce.dest; + return _cache.get(s); } } /** @since 0.8.5 */ - protected void removeCache(String s) { + protected static void removeCache(String s) { synchronized (_cache) { _cache.remove(s); } } /** @since 0.8.1 */ - public void clearCache() { + protected static void clearCache() { synchronized (_cache) { _cache.clear(); } } + + private static class LHM extends LinkedHashMap { + private final int _max; + + public LHM(int max) { + super(max, 0.75f, true); + _max = max; + } + + @Override + protected boolean removeEldestEntry(Map.Entry eldest) { + return size() > _max; + } + } } diff --git a/core/java/src/net/i2p/client/naming/EepGetNamingService.java b/core/java/src/net/i2p/client/naming/EepGetNamingService.java index baf47da86..eeadd6dd4 100644 --- a/core/java/src/net/i2p/client/naming/EepGetNamingService.java +++ b/core/java/src/net/i2p/client/naming/EepGetNamingService.java @@ -39,7 +39,6 @@ public class EepGetNamingService extends DummyNamingService { private final static String PROP_EEPGET_LIST = "i2p.naming.eepget.list"; private final static String DEFAULT_EEPGET_LIST = "http://i2host.i2p/cgi-bin/i2hostquery?"; - private final static Log _log = new Log(EepGetNamingService.class); /** * The naming service should only be constructed and accessed through the diff --git a/core/java/src/net/i2p/client/naming/ExecNamingService.java b/core/java/src/net/i2p/client/naming/ExecNamingService.java index 27aa916af..78c902e4f 100644 --- a/core/java/src/net/i2p/client/naming/ExecNamingService.java +++ b/core/java/src/net/i2p/client/naming/ExecNamingService.java @@ -48,7 +48,6 @@ public class ExecNamingService extends DummyNamingService { private final static String DEFAULT_EXEC_CMD = "/usr/local/bin/i2presolve"; private final static String PROP_SHELL_CMD = "i2p.naming.exec.shell"; private final static String DEFAULT_SHELL_CMD = "/bin/bash"; - private final static Log _log = new Log(ExecNamingService.class); /** * The naming service should only be constructed and accessed through the diff --git a/core/java/src/net/i2p/client/naming/MetaNamingService.java b/core/java/src/net/i2p/client/naming/MetaNamingService.java index 914662adb..2d9cab0d6 100644 --- a/core/java/src/net/i2p/client/naming/MetaNamingService.java +++ b/core/java/src/net/i2p/client/naming/MetaNamingService.java @@ -90,6 +90,7 @@ public class MetaNamingService extends DummyNamingService { @Override public Destination lookup(String hostname, Properties lookupOptions, Properties storedOptions) { + // cache check is in super() Destination d = super.lookup(hostname, null, null); if (d != null) return d; @@ -136,7 +137,10 @@ public class MetaNamingService extends DummyNamingService { public boolean putIfAbsent(String hostname, Destination d, Properties options) { if (_services.isEmpty()) return false; - return _services.get(_services.size() - 1).putIfAbsent(hostname, d, options); + boolean rv = _services.get(_services.size() - 1).putIfAbsent(hostname, d, options); + if (rv) + putCache(hostname, d); + return rv; } /** diff --git a/core/java/src/net/i2p/client/naming/NamingService.java b/core/java/src/net/i2p/client/naming/NamingService.java index e463f91a6..ba26d45ed 100644 --- a/core/java/src/net/i2p/client/naming/NamingService.java +++ b/core/java/src/net/i2p/client/naming/NamingService.java @@ -28,7 +28,7 @@ import net.i2p.util.Log; */ public abstract class NamingService { - private final static Log _log = new Log(NamingService.class); + protected final Log _log; protected final I2PAppContext _context; protected final Set _listeners; protected final Set _updaters; @@ -45,6 +45,7 @@ public abstract class NamingService { */ protected NamingService(I2PAppContext context) { _context = context; + _log = context.logManager().getLog(getClass()); _listeners = new CopyOnWriteArraySet(); _updaters = new CopyOnWriteArraySet(); } @@ -430,7 +431,8 @@ public abstract class NamingService { Constructor con = cls.getConstructor(new Class[] { I2PAppContext.class }); instance = (NamingService)con.newInstance(new Object[] { context }); } catch (Exception ex) { - _log.error("Cannot load naming service " + impl, ex); + Log log = context.logManager().getLog(NamingService.class); + log.error("Cannot load naming service " + impl, ex); instance = new DummyNamingService(context); // fallback } return instance; diff --git a/core/java/src/net/i2p/client/naming/SingleFileNamingService.java b/core/java/src/net/i2p/client/naming/SingleFileNamingService.java index 30722a04f..231ca491c 100644 --- a/core/java/src/net/i2p/client/naming/SingleFileNamingService.java +++ b/core/java/src/net/i2p/client/naming/SingleFileNamingService.java @@ -50,13 +50,13 @@ import net.i2p.util.SecureFileOutputStream; */ public class SingleFileNamingService extends NamingService { - private final static Log _log = new Log(SingleFileNamingService.class); private final File _file; private final ReentrantReadWriteLock _fileLock; /** cached number of entries */ private int _size; /** last write time */ private long _lastWrite; + private volatile boolean _isClosed; public SingleFileNamingService(I2PAppContext context, String filename) { super(context); @@ -171,6 +171,8 @@ public class SingleFileNamingService extends NamingService { BufferedReader in = null; BufferedWriter out = null; try { + if (_isClosed) + return false; File tmp = SecureFile.createTempFile("temp-", ".tmp", _file.getAbsoluteFile().getParentFile()); out = new BufferedWriter(new OutputStreamWriter(new SecureFileOutputStream(tmp), "UTF-8")); if (_file.exists()) { @@ -211,10 +213,12 @@ public class SingleFileNamingService extends NamingService { */ @Override public boolean putIfAbsent(String hostname, Destination d, Properties options) { + OutputStream out = null; if (!getWriteLock()) return false; - OutputStream out = null; try { + if (_isClosed) + return false; // simply check if present, and if not, append try { if (getKey(hostname) != null) @@ -250,13 +254,15 @@ public class SingleFileNamingService extends NamingService { */ @Override public boolean remove(String hostname, Properties options) { - if (!getWriteLock()) - return false; - if (!_file.exists()) - return false; BufferedReader in = null; BufferedWriter out = null; + if (!getWriteLock()) + return false; try { + if (!_file.exists()) + return false; + if (_isClosed) + return false; in = new BufferedReader(new InputStreamReader(new FileInputStream(_file), "UTF-8"), 16*1024); File tmp = SecureFile.createTempFile("temp-", ".tmp", _file.getAbsoluteFile().getParentFile()); out = new BufferedWriter(new OutputStreamWriter(new SecureFileOutputStream(tmp), "UTF-8")); @@ -391,6 +397,16 @@ public class SingleFileNamingService extends NamingService { } } + public void shutdown() { + if (!getWriteLock()) + return; + try { + _isClosed = true; + } finally { + releaseWriteLock(); + } + } + private static boolean rename(File from, File to) { boolean success = false; boolean isWindows = System.getProperty("os.name").startsWith("Win");