From 212c87ffd8b43dfc21af6d8d3570a963f07341c4 Mon Sep 17 00:00:00 2001
From: zzz <zzz@mail.i2p>
Date: Sun, 9 Jan 2011 01:02:17 +0000
Subject: [PATCH] * DataHelper: Speed up and annotate sortStructures()
* RouterInfo: - Don't cache byteified data by default, to save ~1.5 MB
- Don't create empty peers Set, to save ~100KB
---
core/java/src/net/i2p/data/DataHelper.java | 43 +++++++++++----
core/java/src/net/i2p/data/RouterAddress.java | 7 ++-
core/java/src/net/i2p/data/RouterInfo.java | 55 ++++++++++++++-----
3 files changed, 79 insertions(+), 26 deletions(-)
diff --git a/core/java/src/net/i2p/data/DataHelper.java b/core/java/src/net/i2p/data/DataHelper.java
index bd6bf260d9..2e74adce3a 100644
--- a/core/java/src/net/i2p/data/DataHelper.java
+++ b/core/java/src/net/i2p/data/DataHelper.java
@@ -31,6 +31,7 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
+import java.util.Comparator;
import java.util.Date;
import java.util.Iterator;
import java.util.List;
@@ -1040,25 +1041,45 @@ public class DataHelper {
}
/**
- * Sort based on the Hash of the DataStructure
+ * Sort based on the Hash of the DataStructure.
* Warning - relatively slow.
- * Only used by RouterInfo
- * Why? Just because it has to be consistent so signing will work?
+ * WARNING - this sort order must be consistent network-wide, so while the order is arbitrary,
+ * it cannot be changed.
+ * Why? Just because it has to be consistent so signing will work.
* How to spec as returning the same type as the param?
+ * DEPRECATED - Only used by RouterInfo.
*/
public static List<? extends DataStructure> sortStructures(Collection<? extends DataStructure> dataStructures) {
if (dataStructures == null) return Collections.EMPTY_LIST;
- ArrayList<DataStructure> rv = new ArrayList(dataStructures.size());
- TreeMap<String, DataStructure> tm = new TreeMap();
- for (DataStructure struct : dataStructures) {
- tm.put(struct.calculateHash().toString(), struct);
- }
- for (DataStructure struct : tm.values()) {
- rv.add(struct);
- }
+
+ // This used to use Hash.toString(), which is insane, since a change to toString()
+ // would break the whole network. Now use Hash.toBase64().
+ // Note that the Base64 sort order is NOT the same as the raw byte sort order,
+ // despite what you may read elsewhere.
+
+ //ArrayList<DataStructure> rv = new ArrayList(dataStructures.size());
+ //TreeMap<String, DataStructure> tm = new TreeMap();
+ //for (DataStructure struct : dataStructures) {
+ // tm.put(struct.calculateHash().toString(), struct);
+ //}
+ //for (DataStructure struct : tm.values()) {
+ // rv.add(struct);
+ //}
+ ArrayList<DataStructure> rv = new ArrayList(dataStructures);
+ Collections.sort(rv, new DataStructureComparator());
return rv;
}
+ /**
+ * See sortStructures() comments.
+ * @since 0.8.3
+ */
+ private static class DataStructureComparator implements Comparator<DataStructure> {
+ public int compare(DataStructure l, DataStructure r) {
+ return l.calculateHash().toBase64().compareTo(r.calculateHash().toBase64());
+ }
+ }
+
/**
* NOTE: formatDuration2() recommended in most cases for readability
*/
diff --git a/core/java/src/net/i2p/data/RouterAddress.java b/core/java/src/net/i2p/data/RouterAddress.java
index 1b4d6438f5..647104574a 100644
--- a/core/java/src/net/i2p/data/RouterAddress.java
+++ b/core/java/src/net/i2p/data/RouterAddress.java
@@ -131,10 +131,13 @@ public class RouterAddress extends DataStructureImpl {
&& DataHelper.eq(_transportStyle, addr.getTransportStyle());
}
- /** the style should be sufficient, for speed */
+ /**
+ * Just use style and hashCode for speed (expiration is always null).
+ * If we add multiple addresses of the same style, this may need to be changed.
+ */
@Override
public int hashCode() {
- return DataHelper.hashCode(_transportStyle);
+ return DataHelper.hashCode(_transportStyle) ^ _cost;
}
/**
diff --git a/core/java/src/net/i2p/data/RouterInfo.java b/core/java/src/net/i2p/data/RouterInfo.java
index 9c6db01f91..b4aae94b18 100644
--- a/core/java/src/net/i2p/data/RouterInfo.java
+++ b/core/java/src/net/i2p/data/RouterInfo.java
@@ -14,6 +14,7 @@ import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.util.Collection;
+import java.util.Collections;
import java.util.Date;
import java.util.HashSet;
import java.util.Iterator;
@@ -39,7 +40,8 @@ public class RouterInfo extends DatabaseEntry {
private RouterIdentity _identity;
private volatile long _published;
private final Set<RouterAddress> _addresses;
- private final Set<Hash> _peers;
+ /** may be null to save memory, no longer final */
+ private Set<Hash> _peers;
private /* FIXME final FIXME */ Properties _options;
private volatile boolean _validated;
private volatile boolean _isValid;
@@ -47,6 +49,10 @@ public class RouterInfo extends DatabaseEntry {
private volatile byte _byteified[];
private volatile int _hashCode;
private volatile boolean _hashCodeInitialized;
+ /** should we cache the byte and string versions _byteified ? **/
+ private boolean _shouldCache;
+ /** maybe we should check if we are floodfill? */
+ private static final boolean CACHE_ALL = Runtime.getRuntime().maxMemory() > 128*1024*1024l;
public static final String PROP_NETWORK_ID = "netId";
public static final String PROP_CAPABILITIES = "caps";
@@ -58,7 +64,6 @@ public class RouterInfo extends DatabaseEntry {
public RouterInfo() {
_addresses = new HashSet(2);
- _peers = new HashSet(0);
_options = new OrderedProperties();
}
@@ -70,6 +75,7 @@ public class RouterInfo extends DatabaseEntry {
setPeers(old.getPeers());
setOptions(old.getOptions());
setSignature(old.getSignature());
+ // copy over _byteified?
}
public long getDate() {
@@ -105,6 +111,11 @@ public class RouterInfo extends DatabaseEntry {
public void setIdentity(RouterIdentity ident) {
_identity = ident;
resetCache();
+ // We only want to cache the bytes for our own RI, which is frequently written.
+ // To cache for all RIs doubles the RI memory usage.
+ // setIdentity() is only called when we are creating our own RI.
+ // Otherwise, the data is populated with readBytes().
+ _shouldCache = true;
}
/**
@@ -159,6 +170,8 @@ public class RouterInfo extends DatabaseEntry {
* @deprecated Implemented here but unused elsewhere
*/
public Set<Hash> getPeers() {
+ if (_peers == null)
+ return Collections.EMPTY_SET;
return _peers;
}
@@ -169,9 +182,15 @@ public class RouterInfo extends DatabaseEntry {
* @deprecated Implemented here but unused elsewhere
*/
public void setPeers(Set<Hash> peers) {
+ if (peers == null || peers.isEmpty()) {
+ _peers = null;
+ return;
+ }
+ if (_peers == null)
+ _peers = new HashSet(2);
synchronized (_peers) {
_peers.clear();
- if (peers != null) _peers.addAll(peers);
+ _peers.addAll(peers);
}
resetCache();
}
@@ -223,7 +242,6 @@ public class RouterInfo extends DatabaseEntry {
if (_byteified != null) return _byteified;
if (_identity == null) throw new DataFormatException("Router identity isn't set? wtf!");
if (_addresses == null) throw new DataFormatException("Router addressess isn't set? wtf!");
- if (_peers == null) throw new DataFormatException("Router peers isn't set? wtf!");
if (_options == null) throw new DataFormatException("Router options isn't set? wtf!");
long before = Clock.getInstance().now();
@@ -239,6 +257,9 @@ public class RouterInfo extends DatabaseEntry {
DataHelper.writeLong(out, 1, sz);
Collection<RouterAddress> addresses = _addresses;
if (sz > 1)
+ // WARNING this sort algorithm cannot be changed, as it must be consistent
+ // network-wide. The signature is not checked at readin time, but only
+ // later, and the addresses are stored in a Set, not a List.
addresses = (Collection<RouterAddress>) DataHelper.sortStructures(addresses);
for (RouterAddress addr : addresses) {
addr.writeBytes(out);
@@ -248,12 +269,14 @@ public class RouterInfo extends DatabaseEntry {
// answer: they're always empty... they're a placeholder for one particular
// method of trusted links, which isn't implemented in the router
// at the moment, and may not be later.
- // fixme to reduce objects - allow _peers == null
- int psz = _peers.size();
+ int psz = _peers == null ? 0 : _peers.size();
DataHelper.writeLong(out, 1, psz);
if (psz > 0) {
Collection<Hash> peers = _peers;
if (psz > 1)
+ // WARNING this sort algorithm cannot be changed, as it must be consistent
+ // network-wide. The signature is not checked at readin time, but only
+ // later, and the hashes are stored in a Set, not a List.
peers = (Collection<Hash>) DataHelper.sortStructures(peers);
for (Hash peerHash : peers) {
peerHash.writeBytes(out);
@@ -266,7 +289,8 @@ public class RouterInfo extends DatabaseEntry {
byte data[] = out.toByteArray();
long after = Clock.getInstance().now();
_log.debug("getBytes() took " + (after - before) + "ms");
- _byteified = data;
+ if (CACHE_ALL || _shouldCache)
+ _byteified = data;
return data;
}
@@ -466,10 +490,15 @@ public class RouterInfo extends DatabaseEntry {
_addresses.add(address);
}
int numPeers = (int) DataHelper.readLong(in, 1);
- for (int i = 0; i < numPeers; i++) {
- Hash peerIdentityHash = new Hash();
- peerIdentityHash.readBytes(in);
- _peers.add(peerIdentityHash);
+ if (numPeers == 0) {
+ _peers = null;
+ } else {
+ _peers = new HashSet(numPeers);
+ for (int i = 0; i < numPeers; i++) {
+ Hash peerIdentityHash = new Hash();
+ peerIdentityHash.readBytes(in);
+ _peers.add(peerIdentityHash);
+ }
}
_options = DataHelper.readProperties(in);
_signature = new Signature();
@@ -504,7 +533,7 @@ public class RouterInfo extends DatabaseEntry {
&& _published == info.getPublished()
&& DataHelper.eq(_addresses, info.getAddresses())
&& DataHelper.eq(_options, info.getOptions())
- && DataHelper.eq(_peers, info.getPeers());
+ && DataHelper.eq(getPeers(), info.getPeers());
}
@Override
@@ -530,7 +559,7 @@ public class RouterInfo extends DatabaseEntry {
RouterAddress addr = (RouterAddress) iter.next();
buf.append("\n\t\tAddress: ").append(addr);
}
- Set peers = _peers; // getPeers()
+ Set peers = getPeers();
buf.append("\n\tPeers: #: ").append(peers.size());
for (Iterator iter = peers.iterator(); iter.hasNext();) {
Hash hash = (Hash) iter.next();
--
GitLab