You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ws.apache.org by co...@apache.org on 2013/03/12 15:53:02 UTC
svn commit: r1455564 -
/webservices/wss4j/trunk/ws-security-dom/src/main/java/org/apache/wss4j/dom/cache/MemoryReplayCache.java
Author: coheigea
Date: Tue Mar 12 14:53:01 2013
New Revision: 1455564
URL: http://svn.apache.org/r1455564
Log:
[WSS-431] - Performance bottleneck in MemoryReplayCache on high load
- Patch applied, thanks!
Conflicts:
ws-security-dom/src/main/java/org/apache/wss4j/dom/cache/MemoryReplayCache.java
Modified:
webservices/wss4j/trunk/ws-security-dom/src/main/java/org/apache/wss4j/dom/cache/MemoryReplayCache.java
Modified: webservices/wss4j/trunk/ws-security-dom/src/main/java/org/apache/wss4j/dom/cache/MemoryReplayCache.java
URL: http://svn.apache.org/viewvc/webservices/wss4j/trunk/ws-security-dom/src/main/java/org/apache/wss4j/dom/cache/MemoryReplayCache.java?rev=1455564&r1=1455563&r2=1455564&view=diff
==============================================================================
--- webservices/wss4j/trunk/ws-security-dom/src/main/java/org/apache/wss4j/dom/cache/MemoryReplayCache.java (original)
+++ webservices/wss4j/trunk/ws-security-dom/src/main/java/org/apache/wss4j/dom/cache/MemoryReplayCache.java Tue Mar 12 14:53:01 2013
@@ -19,11 +19,16 @@
package org.apache.wss4j.dom.cache;
+import java.util.ArrayList;
import java.util.Collections;
import java.util.Date;
import java.util.HashSet;
import java.util.Iterator;
+import java.util.List;
+import java.util.Map.Entry;
import java.util.Set;
+import java.util.SortedMap;
+import java.util.TreeMap;
/**
* A simple in-memory HashSet based cache to prevent against replay attacks. The default TTL is 5 minutes
@@ -33,8 +38,8 @@ public class MemoryReplayCache implement
public static final long DEFAULT_TTL = 60L * 5L;
public static final long MAX_TTL = DEFAULT_TTL * 12L;
- private final Set<ReplayCacheIdentifier> cache =
- Collections.synchronizedSet(new HashSet<ReplayCacheIdentifier>());
+ private SortedMap<Date, List<String>> cache = new TreeMap<Date, List<String>>();
+ private Set<String> ids = Collections.synchronizedSet(new HashSet<String>());
/**
* Add the given identifier to the cache. It will be cached for a default amount of time.
@@ -53,8 +58,6 @@ public class MemoryReplayCache implement
if (identifier == null || "".equals(identifier)) {
return;
}
- ReplayCacheIdentifier cacheIdentifier = new ReplayCacheIdentifier();
- cacheIdentifier.setIdentifier(identifier);
long ttl = timeToLive;
if (ttl < 0 || ttl > MAX_TTL) {
@@ -64,9 +67,16 @@ public class MemoryReplayCache implement
Date expires = new Date();
long currentTime = expires.getTime();
expires.setTime(currentTime + (ttl * 1000L));
- cacheIdentifier.setExpiry(expires);
- cache.add(cacheIdentifier);
+ synchronized (cache) {
+ List<String> list = cache.get(expires);
+ if (list == null) {
+ list = new ArrayList<String>(1);
+ cache.put(expires, list);
+ }
+ list.add(identifier);
+ }
+ ids.add(identifier);
}
/**
@@ -77,9 +87,7 @@ public class MemoryReplayCache implement
processTokenExpiry();
if (identifier != null && !"".equals(identifier)) {
- ReplayCacheIdentifier cacheIdentifier = new ReplayCacheIdentifier();
- cacheIdentifier.setIdentifier(identifier);
- return cache.contains(cacheIdentifier);
+ return ids.contains(identifier);
}
return false;
}
@@ -87,68 +95,18 @@ public class MemoryReplayCache implement
protected void processTokenExpiry() {
Date current = new Date();
synchronized (cache) {
- Iterator<ReplayCacheIdentifier> iterator = cache.iterator();
- while (iterator.hasNext()) {
- if (iterator.next().getExpiry().before(current)) {
- iterator.remove();
+ Iterator<Entry<Date, List<String>>> it = cache.entrySet().iterator();
+ while (it.hasNext()) {
+ Entry<Date, List<String>> entry = it.next();
+ if (entry.getKey().before(current)) {
+ for (String id : entry.getValue()) {
+ ids.remove(id);
+ }
+ it.remove();
+ } else {
+ break;
}
}
}
}
-
- private static class ReplayCacheIdentifier {
-
- private String identifier;
- private Date expires;
-
- /**
- * Set the (String) identifier
- * @param identifier the (String) identifier
- */
- public void setIdentifier(String identifier) {
- this.identifier = identifier;
- }
-
- /**
- * Set when this identifier is to be removed from the cache
- * @param expires when this identifier is to be removed from the cache
- */
- public void setExpiry(Date expires) {
- this.expires = expires;
- }
-
- /**
- * Get when this identifier is to be removed from the cache
- * @return when this identifier is to be removed from the cache
- */
- public Date getExpiry() {
- return expires;
- }
-
- @Override
- public boolean equals(Object o) {
- if (this == o) {
- return true;
- }
- if (!(o instanceof ReplayCacheIdentifier)) {
- return false;
- }
-
- ReplayCacheIdentifier replayCacheIdentifier = (ReplayCacheIdentifier)o;
-
- if (identifier == null && replayCacheIdentifier.identifier != null) {
- return false;
- } else if (identifier != null && !identifier.equals(replayCacheIdentifier.identifier)) {
- return false;
- }
-
- return true;
- }
-
- @Override
- public int hashCode() {
- return identifier != null ? identifier.hashCode() : 0;
- }
- }
-
}
Re: svn commit: r1455564 - /webservices/wss4j/trunk/ws-security-dom/src/main/java/org/apache/wss4j/dom/cache/MemoryReplayCache.java
Posted by Colm O hEigeartaigh <co...@apache.org>.
Could you log a JIRA to capture this?
Thanks,
Colm.
On Tue, Mar 12, 2013 at 5:48 PM, Daniel Kulp <dk...@apache.org> wrote:
>
> Colm,
>
> On trunk where we've dropped support for Java 5, we could likely use a
> ConcurrentSkipListSet and Map instead of the synchronized collections.
> Just a thought. :-)
>
> Dan
>
>
>
>
>
> On Mar 12, 2013, at 10:53 AM, coheigea@apache.org wrote:
>
> > Author: coheigea
> > Date: Tue Mar 12 14:53:01 2013
> > New Revision: 1455564
> >
> > URL: http://svn.apache.org/r1455564
> > Log:
> > [WSS-431] - Performance bottleneck in MemoryReplayCache on high load
> > - Patch applied, thanks!
> >
> >
> > Conflicts:
> >
> ws-security-dom/src/main/java/org/apache/wss4j/dom/cache/MemoryReplayCache.java
> >
> > Modified:
> >
> webservices/wss4j/trunk/ws-security-dom/src/main/java/org/apache/wss4j/dom/cache/MemoryReplayCache.java
> >
> > Modified:
> webservices/wss4j/trunk/ws-security-dom/src/main/java/org/apache/wss4j/dom/cache/MemoryReplayCache.java
> > URL:
> http://svn.apache.org/viewvc/webservices/wss4j/trunk/ws-security-dom/src/main/java/org/apache/wss4j/dom/cache/MemoryReplayCache.java?rev=1455564&r1=1455563&r2=1455564&view=diff
> >
> ==============================================================================
> > ---
> webservices/wss4j/trunk/ws-security-dom/src/main/java/org/apache/wss4j/dom/cache/MemoryReplayCache.java
> (original)
> > +++
> webservices/wss4j/trunk/ws-security-dom/src/main/java/org/apache/wss4j/dom/cache/MemoryReplayCache.java
> Tue Mar 12 14:53:01 2013
> > @@ -19,11 +19,16 @@
> >
> > package org.apache.wss4j.dom.cache;
> >
> > +import java.util.ArrayList;
> > import java.util.Collections;
> > import java.util.Date;
> > import java.util.HashSet;
> > import java.util.Iterator;
> > +import java.util.List;
> > +import java.util.Map.Entry;
> > import java.util.Set;
> > +import java.util.SortedMap;
> > +import java.util.TreeMap;
> >
> > /**
> > * A simple in-memory HashSet based cache to prevent against replay
> attacks. The default TTL is 5 minutes
> > @@ -33,8 +38,8 @@ public class MemoryReplayCache implement
> >
> > public static final long DEFAULT_TTL = 60L * 5L;
> > public static final long MAX_TTL = DEFAULT_TTL * 12L;
> > - private final Set<ReplayCacheIdentifier> cache =
> > - Collections.synchronizedSet(new
> HashSet<ReplayCacheIdentifier>());
> > + private SortedMap<Date, List<String>> cache = new TreeMap<Date,
> List<String>>();
> > + private Set<String> ids = Collections.synchronizedSet(new
> HashSet<String>());
> >
> > /**
> > * Add the given identifier to the cache. It will be cached for a
> default amount of time.
> > @@ -53,8 +58,6 @@ public class MemoryReplayCache implement
> > if (identifier == null || "".equals(identifier)) {
> > return;
> > }
> > - ReplayCacheIdentifier cacheIdentifier = new
> ReplayCacheIdentifier();
> > - cacheIdentifier.setIdentifier(identifier);
> >
> > long ttl = timeToLive;
> > if (ttl < 0 || ttl > MAX_TTL) {
> > @@ -64,9 +67,16 @@ public class MemoryReplayCache implement
> > Date expires = new Date();
> > long currentTime = expires.getTime();
> > expires.setTime(currentTime + (ttl * 1000L));
> > - cacheIdentifier.setExpiry(expires);
> >
> > - cache.add(cacheIdentifier);
> > + synchronized (cache) {
> > + List<String> list = cache.get(expires);
> > + if (list == null) {
> > + list = new ArrayList<String>(1);
> > + cache.put(expires, list);
> > + }
> > + list.add(identifier);
> > + }
> > + ids.add(identifier);
> > }
> >
> > /**
> > @@ -77,9 +87,7 @@ public class MemoryReplayCache implement
> > processTokenExpiry();
> >
> > if (identifier != null && !"".equals(identifier)) {
> > - ReplayCacheIdentifier cacheIdentifier = new
> ReplayCacheIdentifier();
> > - cacheIdentifier.setIdentifier(identifier);
> > - return cache.contains(cacheIdentifier);
> > + return ids.contains(identifier);
> > }
> > return false;
> > }
> > @@ -87,68 +95,18 @@ public class MemoryReplayCache implement
> > protected void processTokenExpiry() {
> > Date current = new Date();
> > synchronized (cache) {
> > - Iterator<ReplayCacheIdentifier> iterator = cache.iterator();
> > - while (iterator.hasNext()) {
> > - if (iterator.next().getExpiry().before(current)) {
> > - iterator.remove();
> > + Iterator<Entry<Date, List<String>>> it =
> cache.entrySet().iterator();
> > + while (it.hasNext()) {
> > + Entry<Date, List<String>> entry = it.next();
> > + if (entry.getKey().before(current)) {
> > + for (String id : entry.getValue()) {
> > + ids.remove(id);
> > + }
> > + it.remove();
> > + } else {
> > + break;
> > }
> > }
> > }
> > }
> > -
> > - private static class ReplayCacheIdentifier {
> > -
> > - private String identifier;
> > - private Date expires;
> > -
> > - /**
> > - * Set the (String) identifier
> > - * @param identifier the (String) identifier
> > - */
> > - public void setIdentifier(String identifier) {
> > - this.identifier = identifier;
> > - }
> > -
> > - /**
> > - * Set when this identifier is to be removed from the cache
> > - * @param expires when this identifier is to be removed from
> the cache
> > - */
> > - public void setExpiry(Date expires) {
> > - this.expires = expires;
> > - }
> > -
> > - /**
> > - * Get when this identifier is to be removed from the cache
> > - * @return when this identifier is to be removed from the cache
> > - */
> > - public Date getExpiry() {
> > - return expires;
> > - }
> > -
> > - @Override
> > - public boolean equals(Object o) {
> > - if (this == o) {
> > - return true;
> > - }
> > - if (!(o instanceof ReplayCacheIdentifier)) {
> > - return false;
> > - }
> > -
> > - ReplayCacheIdentifier replayCacheIdentifier =
> (ReplayCacheIdentifier)o;
> > -
> > - if (identifier == null && replayCacheIdentifier.identifier
> != null) {
> > - return false;
> > - } else if (identifier != null &&
> !identifier.equals(replayCacheIdentifier.identifier)) {
> > - return false;
> > - }
> > -
> > - return true;
> > - }
> > -
> > - @Override
> > - public int hashCode() {
> > - return identifier != null ? identifier.hashCode() : 0;
> > - }
> > - }
> > -
> > }
> >
> >
>
> --
> Daniel Kulp
> dkulp@apache.org - http://dankulp.com/blog
> Talend Community Coder - http://coders.talend.com
>
>
--
Colm O hEigeartaigh
Talend Community Coder
http://coders.talend.com
Re: svn commit: r1455564 - /webservices/wss4j/trunk/ws-security-dom/src/main/java/org/apache/wss4j/dom/cache/MemoryReplayCache.java
Posted by Daniel Kulp <dk...@apache.org>.
Colm,
On trunk where we've dropped support for Java 5, we could likely use a ConcurrentSkipListSet and Map instead of the synchronized collections. Just a thought. :-)
Dan
On Mar 12, 2013, at 10:53 AM, coheigea@apache.org wrote:
> Author: coheigea
> Date: Tue Mar 12 14:53:01 2013
> New Revision: 1455564
>
> URL: http://svn.apache.org/r1455564
> Log:
> [WSS-431] - Performance bottleneck in MemoryReplayCache on high load
> - Patch applied, thanks!
>
>
> Conflicts:
> ws-security-dom/src/main/java/org/apache/wss4j/dom/cache/MemoryReplayCache.java
>
> Modified:
> webservices/wss4j/trunk/ws-security-dom/src/main/java/org/apache/wss4j/dom/cache/MemoryReplayCache.java
>
> Modified: webservices/wss4j/trunk/ws-security-dom/src/main/java/org/apache/wss4j/dom/cache/MemoryReplayCache.java
> URL: http://svn.apache.org/viewvc/webservices/wss4j/trunk/ws-security-dom/src/main/java/org/apache/wss4j/dom/cache/MemoryReplayCache.java?rev=1455564&r1=1455563&r2=1455564&view=diff
> ==============================================================================
> --- webservices/wss4j/trunk/ws-security-dom/src/main/java/org/apache/wss4j/dom/cache/MemoryReplayCache.java (original)
> +++ webservices/wss4j/trunk/ws-security-dom/src/main/java/org/apache/wss4j/dom/cache/MemoryReplayCache.java Tue Mar 12 14:53:01 2013
> @@ -19,11 +19,16 @@
>
> package org.apache.wss4j.dom.cache;
>
> +import java.util.ArrayList;
> import java.util.Collections;
> import java.util.Date;
> import java.util.HashSet;
> import java.util.Iterator;
> +import java.util.List;
> +import java.util.Map.Entry;
> import java.util.Set;
> +import java.util.SortedMap;
> +import java.util.TreeMap;
>
> /**
> * A simple in-memory HashSet based cache to prevent against replay attacks. The default TTL is 5 minutes
> @@ -33,8 +38,8 @@ public class MemoryReplayCache implement
>
> public static final long DEFAULT_TTL = 60L * 5L;
> public static final long MAX_TTL = DEFAULT_TTL * 12L;
> - private final Set<ReplayCacheIdentifier> cache =
> - Collections.synchronizedSet(new HashSet<ReplayCacheIdentifier>());
> + private SortedMap<Date, List<String>> cache = new TreeMap<Date, List<String>>();
> + private Set<String> ids = Collections.synchronizedSet(new HashSet<String>());
>
> /**
> * Add the given identifier to the cache. It will be cached for a default amount of time.
> @@ -53,8 +58,6 @@ public class MemoryReplayCache implement
> if (identifier == null || "".equals(identifier)) {
> return;
> }
> - ReplayCacheIdentifier cacheIdentifier = new ReplayCacheIdentifier();
> - cacheIdentifier.setIdentifier(identifier);
>
> long ttl = timeToLive;
> if (ttl < 0 || ttl > MAX_TTL) {
> @@ -64,9 +67,16 @@ public class MemoryReplayCache implement
> Date expires = new Date();
> long currentTime = expires.getTime();
> expires.setTime(currentTime + (ttl * 1000L));
> - cacheIdentifier.setExpiry(expires);
>
> - cache.add(cacheIdentifier);
> + synchronized (cache) {
> + List<String> list = cache.get(expires);
> + if (list == null) {
> + list = new ArrayList<String>(1);
> + cache.put(expires, list);
> + }
> + list.add(identifier);
> + }
> + ids.add(identifier);
> }
>
> /**
> @@ -77,9 +87,7 @@ public class MemoryReplayCache implement
> processTokenExpiry();
>
> if (identifier != null && !"".equals(identifier)) {
> - ReplayCacheIdentifier cacheIdentifier = new ReplayCacheIdentifier();
> - cacheIdentifier.setIdentifier(identifier);
> - return cache.contains(cacheIdentifier);
> + return ids.contains(identifier);
> }
> return false;
> }
> @@ -87,68 +95,18 @@ public class MemoryReplayCache implement
> protected void processTokenExpiry() {
> Date current = new Date();
> synchronized (cache) {
> - Iterator<ReplayCacheIdentifier> iterator = cache.iterator();
> - while (iterator.hasNext()) {
> - if (iterator.next().getExpiry().before(current)) {
> - iterator.remove();
> + Iterator<Entry<Date, List<String>>> it = cache.entrySet().iterator();
> + while (it.hasNext()) {
> + Entry<Date, List<String>> entry = it.next();
> + if (entry.getKey().before(current)) {
> + for (String id : entry.getValue()) {
> + ids.remove(id);
> + }
> + it.remove();
> + } else {
> + break;
> }
> }
> }
> }
> -
> - private static class ReplayCacheIdentifier {
> -
> - private String identifier;
> - private Date expires;
> -
> - /**
> - * Set the (String) identifier
> - * @param identifier the (String) identifier
> - */
> - public void setIdentifier(String identifier) {
> - this.identifier = identifier;
> - }
> -
> - /**
> - * Set when this identifier is to be removed from the cache
> - * @param expires when this identifier is to be removed from the cache
> - */
> - public void setExpiry(Date expires) {
> - this.expires = expires;
> - }
> -
> - /**
> - * Get when this identifier is to be removed from the cache
> - * @return when this identifier is to be removed from the cache
> - */
> - public Date getExpiry() {
> - return expires;
> - }
> -
> - @Override
> - public boolean equals(Object o) {
> - if (this == o) {
> - return true;
> - }
> - if (!(o instanceof ReplayCacheIdentifier)) {
> - return false;
> - }
> -
> - ReplayCacheIdentifier replayCacheIdentifier = (ReplayCacheIdentifier)o;
> -
> - if (identifier == null && replayCacheIdentifier.identifier != null) {
> - return false;
> - } else if (identifier != null && !identifier.equals(replayCacheIdentifier.identifier)) {
> - return false;
> - }
> -
> - return true;
> - }
> -
> - @Override
> - public int hashCode() {
> - return identifier != null ? identifier.hashCode() : 0;
> - }
> - }
> -
> }
>
>
--
Daniel Kulp
dkulp@apache.org - http://dankulp.com/blog
Talend Community Coder - http://coders.talend.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ws.apache.org
For additional commands, e-mail: dev-help@ws.apache.org