Skip to content

O
Openfire
Commit 9834cdfb authored by Gaston Dombiak's avatar Gaston Dombiak Committed by gato
Browse files
Options

Improved performance by not using ReentrantReadWriteLock. JM-621 

git-svn-id: http://svn.igniterealtime.org/svn/repos/wildfire/trunk@3672 b35dd754-fafc-0310-a699-88a17e54d16e
parent 6ff3e09e
Hide whitespace changes
Inline Side-by-side
Showing with 68 additions and 87 deletions
src/java/org/jivesoftware/wildfire/spi/RoutingTableImpl.java
View file @ 9834cdfb
...@@ -11,21 +11,19 @@ ...@@ -11,21 +11,19 @@
package org.jivesoftware.wildfire.spi; package org.jivesoftware.wildfire.spi;
import org.jivesoftware.util.Log;
import org.jivesoftware.wildfire.*; import org.jivesoftware.wildfire.*;
import org.jivesoftware.wildfire.component.InternalComponentManager; import org.jivesoftware.wildfire.component.InternalComponentManager;
import org.jivesoftware.wildfire.container.BasicModule; import org.jivesoftware.wildfire.container.BasicModule;
import org.jivesoftware.wildfire.server.OutgoingSessionPromise; import org.jivesoftware.wildfire.server.OutgoingSessionPromise;
import org.jivesoftware.util.Log;
import org.xmpp.packet.JID; import org.xmpp.packet.JID;
import java.util.*; import java.util.*;
import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
/** /**
* <p>Uses simple hashtables for table storage.</p> * <p>Uses simple Maps for table storage.</p>
* <p>Leaves in the tree are indicated by a PacketHandler, while branches are stored in hashtables. * <p>Leaves in the tree are indicated by a PacketHandler, while branches are stored in Maps.
* Traverse the tree according to an XMPPAddress' fields (host -> name -> resource) and when you * Traverse the tree according to an XMPPAddress' fields (host -> name -> resource) and when you
* hit a PacketHandler, you have found the handler for that node and all sub-nodes. </p> * hit a PacketHandler, you have found the handler for that node and all sub-nodes. </p>
* *
...@@ -38,10 +36,6 @@ public class RoutingTableImpl extends BasicModule implements RoutingTable { ...@@ -38,10 +36,6 @@ public class RoutingTableImpl extends BasicModule implements RoutingTable {
*/ */
private Map routes = new ConcurrentHashMap(); private Map routes = new ConcurrentHashMap();
/**
* locks access to the routing tale
*/
private ReadWriteLock routeLock = new ReentrantReadWriteLock();
private String serverName; private String serverName;
private InternalComponentManager componentManager; private InternalComponentManager componentManager;
...@@ -55,27 +49,37 @@ public class RoutingTableImpl extends BasicModule implements RoutingTable { ...@@ -55,27 +49,37 @@ public class RoutingTableImpl extends BasicModule implements RoutingTable {
String nodeJID = node.getNode() == null ? "" : node.getNode(); String nodeJID = node.getNode() == null ? "" : node.getNode();
String resourceJID = node.getResource() == null ? "" : node.getResource(); String resourceJID = node.getResource() == null ? "" : node.getResource();
routeLock.writeLock().lock(); if (destination instanceof ClientSession) {
try { Object nameRoutes = routes.get(node.getDomain());
if (destination instanceof ClientSession) { if (nameRoutes == null) {
Object nameRoutes = routes.get(node.getDomain()); // No route to the requested domain. Create a new entry in the table
if (nameRoutes == null) { synchronized (node.getDomain().intern()) {
nameRoutes = new Hashtable(); // Check again if a route exists now that we have a lock
routes.put(node.getDomain(), nameRoutes); nameRoutes = routes.get(node.getDomain());
} if (nameRoutes == null) {
Object resourceRoutes = ((Hashtable)nameRoutes).get(nodeJID); // Still nothing so create a new entry in the map for domain
if (resourceRoutes == null) { nameRoutes = new ConcurrentHashMap();
resourceRoutes = new Hashtable(); routes.put(node.getDomain(), nameRoutes);
((Hashtable)nameRoutes).put(nodeJID, resourceRoutes); }
} }
((Hashtable)resourceRoutes).put(resourceJID, destination);
} }
else { // Check if there is something associated with the node of the JID
routes.put(node.getDomain(), destination); Object resourceRoutes = ((Map) nameRoutes).get(nodeJID);
if (resourceRoutes == null) {
// Nothing was found so create a new entry for this node (a.k.a. user)
synchronized (nodeJID.intern()) {
resourceRoutes = ((Map) nameRoutes).get(nodeJID);
if (resourceRoutes == null) {
resourceRoutes = new ConcurrentHashMap();
((Map) nameRoutes).put(nodeJID, resourceRoutes);
}
}
} }
// Add the connected resource to the node's Map
((Map) resourceRoutes).put(resourceJID, destination);
} }
finally { else {
routeLock.writeLock().unlock(); routes.put(node.getDomain(), destination);
} }
} }
...@@ -99,19 +103,18 @@ public class RoutingTableImpl extends BasicModule implements RoutingTable { ...@@ -99,19 +103,18 @@ public class RoutingTableImpl extends BasicModule implements RoutingTable {
return OutgoingSessionPromise.getInstance(); return OutgoingSessionPromise.getInstance();
} }
routeLock.readLock().lock();
try { try {
Object nameRoutes = routes.get(domain); Object nameRoutes = routes.get(domain);
if (nameRoutes instanceof ChannelHandler) { if (nameRoutes instanceof ChannelHandler) {
route = (RoutableChannelHandler)nameRoutes; route = (RoutableChannelHandler) nameRoutes;
} }
else if (nameRoutes != null) { else if (nameRoutes != null) {
Object resourceRoutes = ((Hashtable)nameRoutes).get(node); Object resourceRoutes = ((Map) nameRoutes).get(node);
if (resourceRoutes instanceof ChannelHandler) { if (resourceRoutes instanceof ChannelHandler) {
route = (RoutableChannelHandler)resourceRoutes; route = (RoutableChannelHandler) resourceRoutes;
} }
else if (resourceRoutes != null) { else if (resourceRoutes != null) {
route = (RoutableChannelHandler) ((Hashtable)resourceRoutes).get(resource); route = (RoutableChannelHandler) ((Map) resourceRoutes).get(resource);
} }
else { else {
route = null; route = null;
...@@ -120,12 +123,9 @@ public class RoutingTableImpl extends BasicModule implements RoutingTable { ...@@ -120,12 +123,9 @@ public class RoutingTableImpl extends BasicModule implements RoutingTable {
} }
catch (Exception e) { catch (Exception e) {
if (Log.isDebugEnabled()) { if (Log.isDebugEnabled()) {
Log.debug("Route not found for JID: "+ jid, e); Log.debug("Route not found for JID: " + jid, e);
} }
} }
finally {
routeLock.readLock().unlock();
}
return route; return route;
} }
...@@ -140,51 +140,37 @@ public class RoutingTableImpl extends BasicModule implements RoutingTable { ...@@ -140,51 +140,37 @@ public class RoutingTableImpl extends BasicModule implements RoutingTable {
} }
LinkedList list = null; LinkedList list = null;
routeLock.readLock().lock(); Object nameRoutes = routes.get(node.getDomain());
try { if (nameRoutes != null) {
if (node == null || node.getDomain() == null) { if (nameRoutes instanceof ChannelHandler) {
list = new LinkedList();
list.add(nameRoutes);
}
else if (node.getNode() == null) {
list = new LinkedList(); list = new LinkedList();
getRoutes(list, routes); getRoutes(list, (Map) nameRoutes);
} }
else { else {
Object nameRoutes = routes.get(node.getDomain()); Object resourceRoutes = ((Map) nameRoutes).get(node.getNode());
if (nameRoutes != null) { if (resourceRoutes != null) {
if (nameRoutes instanceof ChannelHandler) { if (resourceRoutes instanceof ChannelHandler) {
list = new LinkedList(); list = new LinkedList();
list.add(nameRoutes); list.add(resourceRoutes);
} }
else if (node.getNode() == null) { else if (node.getResource() == null || node.getResource().length() == 0) {
list = new LinkedList(); list = new LinkedList();
getRoutes(list, (Hashtable)nameRoutes); getRoutes(list, (Map) resourceRoutes);
} }
else { else {
Object resourceRoutes = Object entry = ((Map) resourceRoutes).get(node.getResource());
((Hashtable)nameRoutes).get(node.getNode()); if (entry != null) {
if (resourceRoutes != null) { list = new LinkedList();
if (resourceRoutes instanceof ChannelHandler) { list.add(entry);
list = new LinkedList();
list.add(resourceRoutes);
}
else if (node.getResource() == null || node.getResource().length() == 0) {
list = new LinkedList();
getRoutes(list, (Hashtable)resourceRoutes);
}
else {
Object entry =
((Hashtable)resourceRoutes).get(node.getResource());
if (entry != null) {
list = new LinkedList();
list.add(entry);
}
}
} }
} }
} }
} }
} }
finally {
routeLock.readLock().unlock();
}
if (list == null) { if (list == null) {
return Collections.EMPTY_LIST.iterator(); return Collections.EMPTY_LIST.iterator();
} }
...@@ -194,10 +180,10 @@ public class RoutingTableImpl extends BasicModule implements RoutingTable { ...@@ -194,10 +180,10 @@ public class RoutingTableImpl extends BasicModule implements RoutingTable {
} }
/** /**
* <p>Recursive method to iterate through the given table (and any embedded tables) * Recursive method to iterate through the given table (and any embedded map)
* and stuff non-Hashtable values into the given list.</p> * and stuff non-Map values into the given list.<p>
* <p>There should be no recursion problems since *
* the routing table is at most 3 levels deep.</p> * There should be no recursion problems since the routing table is at most 3 levels deep.
* *
* @param list The list to stuff entries into * @param list The list to stuff entries into
* @param table The hashtable who's values should be entered into the list * @param table The hashtable who's values should be entered into the list
...@@ -206,8 +192,8 @@ public class RoutingTableImpl extends BasicModule implements RoutingTable { ...@@ -206,8 +192,8 @@ public class RoutingTableImpl extends BasicModule implements RoutingTable {
Iterator entryIter = table.values().iterator(); Iterator entryIter = table.values().iterator();
while (entryIter.hasNext()) { while (entryIter.hasNext()) {
Object entry = entryIter.next(); Object entry = entryIter.next();
if (entry instanceof Hashtable) { if (entry instanceof ConcurrentHashMap) {
getRoutes(list, (Hashtable)entry); getRoutes(list, (Map)entry);
} }
else { else {
// Do not include the same entry many times. This could be the case when the same // Do not include the same entry many times. This could be the case when the same
...@@ -220,7 +206,7 @@ public class RoutingTableImpl extends BasicModule implements RoutingTable { ...@@ -220,7 +206,7 @@ public class RoutingTableImpl extends BasicModule implements RoutingTable {
} }
public ChannelHandler getBestRoute(JID node) { public ChannelHandler getBestRoute(JID node) {
ChannelHandler route = route = getRoute(node); ChannelHandler route = getRoute(node);
if (route == null) { if (route == null) {
// Try looking for a route based on the bare JID // Try looking for a route based on the bare JID
String nodeJID = node.getNode() == null ? "" : node.getNode(); String nodeJID = node.getNode() == null ? "" : node.getNode();
...@@ -235,25 +221,23 @@ public class RoutingTableImpl extends BasicModule implements RoutingTable { ...@@ -235,25 +221,23 @@ public class RoutingTableImpl extends BasicModule implements RoutingTable {
String nodeJID = node.getNode() == null ? "" : node.getNode(); String nodeJID = node.getNode() == null ? "" : node.getNode();
String resourceJID = node.getResource() == null ? "" : node.getResource(); String resourceJID = node.getResource() == null ? "" : node.getResource();
routeLock.writeLock().lock();
try { try {
Object nameRoutes = routes.get(node.getDomain()); Object nameRoutes = routes.get(node.getDomain());
if (nameRoutes instanceof Hashtable) { if (nameRoutes instanceof ConcurrentHashMap) {
Object resourceRoutes = ((Hashtable)nameRoutes).get(nodeJID); Object resourceRoutes = ((Map) nameRoutes).get(nodeJID);
if (resourceRoutes instanceof Hashtable) { if (resourceRoutes instanceof ConcurrentHashMap) {
// Remove the requested resource for this user // Remove the requested resource for this user
route = (ChannelHandler) ((Hashtable)resourceRoutes).remove(resourceJID); route = (ChannelHandler) ((Map) resourceRoutes).remove(resourceJID);
if (((Hashtable)resourceRoutes).isEmpty()) { if (((Map) resourceRoutes).isEmpty()) {
((Hashtable)nameRoutes).remove(nodeJID); ((Map) nameRoutes).remove(nodeJID);
if (((Hashtable)nameRoutes).isEmpty()) { if (((Map) nameRoutes).isEmpty()) {
routes.remove(node.getDomain()); routes.remove(node.getDomain());
} }
} }
} }
else { else {
// Remove the unique route to this node // Remove the unique route to this node
((Hashtable)nameRoutes).remove(nodeJID); ((Map) nameRoutes).remove(nodeJID);
} }
} }
else if (nameRoutes != null) { else if (nameRoutes != null) {
...@@ -268,9 +252,6 @@ public class RoutingTableImpl extends BasicModule implements RoutingTable { ...@@ -268,9 +252,6 @@ public class RoutingTableImpl extends BasicModule implements RoutingTable {
catch (Exception e) { catch (Exception e) {
Log.error("Error removing route", e); Log.error("Error removing route", e);
} }
finally {
routeLock.writeLock().unlock();
}
return route; return route;
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment

Technounit-GROUP 2023