Merge pull request wildfly#9163 from pferraro/master

WFLY-6996 High CPU usage when using attribute web session granularity

clustering/marshalling/jboss/src/main/java/org/wildfly/clustering/marshalling/jboss/SetExternalizer.java

@@ -0,0 +1,98 @@
+/*
+ * JBoss, Home of Professional Open Source.
+ * Copyright 2016, Red Hat, Inc., and individual contributors
+ * as indicated by the @author tags. See the copyright.txt file in the
+ * distribution for a full listing of individual contributors.
+ *
+ * This is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as
+ * published by the Free Software Foundation; either version 2.1 of
+ * the License, or (at your option) any later version.
+ *
+ * This software is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this software; if not, write to the Free
+ * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA, or see the FSF site: http://www.fsf.org.
+ */
+
+package org.wildfly.clustering.marshalling.jboss;
+
+import java.io.IOException;
+import java.io.ObjectInput;
+import java.io.ObjectOutput;
+import java.util.HashSet;
+import java.util.LinkedHashSet;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.IntFunction;
+
+import org.wildfly.clustering.marshalling.Externalizer;
+
+/**
+ * Externalizers for implementations of {@link Set}.
+ * @author Paul Ferraro
+ */
+public class SetExternalizer<T extends Set<Object>> implements Externalizer<T> {
+
+    private final Class<T> targetClass;
+    private final IntFunction<T> factory;
+
+    @SuppressWarnings({ "unchecked", "rawtypes" })
+    SetExternalizer(Class targetClass, IntFunction<T> factory) {
+        this.targetClass = targetClass;
+        this.factory = factory;
+    }
+
+    @Override
+    public void writeObject(ObjectOutput output, T set) throws IOException {
+        writeSet(output, set);
+    }
+
+    static <T extends Set<Object>> void writeSet(ObjectOutput output, T set) throws IOException {
+        IndexExternalizer.VARIABLE.writeData(output, set.size());
+        for (Object e : set) {
+            output.writeObject(e);
+        }
+    }
+
+    @Override
+    public T readObject(ObjectInput input) throws IOException, ClassNotFoundException {
+        int size = IndexExternalizer.VARIABLE.readData(input);
+        return readSet(input, this.factory.apply(size), size);
+    }
+
+    static <T extends Set<Object>> T readSet(ObjectInput input, T set, int size) throws IOException, ClassNotFoundException {
+        for (int i = 0; i < size; ++i) {
+            set.add(input.readObject());
+        }
+        return set;
+    }
+
+    @Override
+    public Class<? extends T> getTargetClass() {
+        return this.targetClass;
+    }
+
+    public static class ConcurrentHashSetExternalizer extends SetExternalizer<ConcurrentHashMap.KeySetView<Object, Boolean>> {
+        public ConcurrentHashSetExternalizer() {
+            super(ConcurrentHashMap.KeySetView.class, capacity -> ConcurrentHashMap.newKeySet(capacity));
+        }
+    }
+
+    public static class HashSetExternalizer extends SetExternalizer<HashSet<Object>> {
+        public HashSetExternalizer() {
+            super(HashSet.class, HashSet::new);
+        }
+    }
+
+    public static class LinkedHashSetExternalizer extends SetExternalizer<LinkedHashSet<Object>> {
+        public LinkedHashSetExternalizer() {
+            super(LinkedHashSet.class, LinkedHashSet::new);
+        }
+    }
+}

clustering/marshalling/jboss/src/main/resources/META-INF/services/org.wildfly.clustering.marshalling.Externalizer

@@ -16,6 +16,9 @@ org.wildfly.clustering.marshalling.jboss.MapExternalizer$ConcurrentHashMapExtern
 org.wildfly.clustering.marshalling.jboss.MapExternalizer$HashMapExternalizer
 org.wildfly.clustering.marshalling.jboss.MapExternalizer$LinkedHashMapExternalizer
 org.wildfly.clustering.marshalling.jboss.SimpleMarshalledValueExternalizer
+org.wildfly.clustering.marshalling.jboss.SetExternalizer$ConcurrentHashSetExternalizer
+org.wildfly.clustering.marshalling.jboss.SetExternalizer$HashSetExternalizer
+org.wildfly.clustering.marshalling.jboss.SetExternalizer$LinkedHashSetExternalizer
 org.wildfly.clustering.marshalling.jboss.SortedMapExternalizer$ConcurrentSkipListMapExternalizer
 org.wildfly.clustering.marshalling.jboss.SortedMapExternalizer$TreeMapExternalizer
 org.wildfly.clustering.marshalling.jboss.SortedSetExternalizer$ConcurrentSkipListSetExternalizer

clustering/web/infinispan/src/main/java/org/wildfly/clustering/web/infinispan/session/InfinispanSessionManagerFactory.java

@@ -174,7 +174,7 @@ private SessionAttributesFactory<?> createSessionAttributesFactory(CacheProperti
 
         switch (this.config.getSessionManagerFactoryConfiguration().getAttributePersistenceStrategy()) {
             case FINE: {
-                return new FineSessionAttributesFactory(this.config.getCache(), new MarshalledValueMarshaller<>(factory, context), properties);
+                return new FineSessionAttributesFactory(this.config.getCache(), this.config.getCache(), new MarshalledValueMarshaller<>(factory, context), properties);
             }
             case COARSE: {
                 return new CoarseSessionAttributesFactory(this.config.getCache(), new MarshalledValueMarshaller<>(factory, context), properties);

clustering/web/infinispan/src/main/java/org/wildfly/clustering/web/infinispan/session/coarse/CoarseSessionAttributesFactory.java

@@ -26,6 +26,7 @@
 import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Function;
 
 import org.infinispan.Cache;
 import org.infinispan.context.Flag;
@@ -59,21 +60,18 @@ public CoarseSessionAttributesFactory(Cache<SessionAttributesKey, MarshalledValu
 
     @Override
     public Map.Entry<Map<String, Object>, MarshalledValue<Map<String, Object>, MarshallingContext>> createValue(String id, Void context) {
-        SessionAttributesKey attributesKey = new SessionAttributesKey(id);
-        MarshalledValue<Map<String, Object>, MarshallingContext> value = this.cache.getAdvancedCache().withFlags(Flag.FORCE_SYNCHRONOUS).computeIfAbsent(attributesKey, key -> this.marshaller.write(this.properties.isLockOnRead() ? new HashMap<>() : new ConcurrentHashMap<>()));
-        try {
-            Map<String, Object> attributes = this.marshaller.read(value);
-            return new SimpleImmutableEntry<>(attributes, value);
-        } catch (InvalidSerializedFormException e) {
-            InfinispanWebLogger.ROOT_LOGGER.failedToActivateSession(e, id);
-            this.cache.getAdvancedCache().withFlags(Flag.IGNORE_RETURN_VALUES).remove(attributesKey);
-            return this.createValue(id, context);
-        }
+        Map.Entry<Map<String, Object>, MarshalledValue<Map<String, Object>, MarshallingContext>> value = this.getValue(id, key -> this.cache.getAdvancedCache().withFlags(Flag.FORCE_SYNCHRONOUS).computeIfAbsent(key, k -> this.marshaller.write(this.properties.isLockOnRead() ? new HashMap<>() : new ConcurrentHashMap<>())));
+        return (value != null) ? value : this.createValue(id, context);
     }
 
     @Override
     public Map.Entry<Map<String, Object>, MarshalledValue<Map<String, Object>, MarshallingContext>> findValue(String id) {
-        MarshalledValue<Map<String, Object>, MarshallingContext> value = this.cache.get(new SessionAttributesKey(id));
+        return this.getValue(id, key -> this.cache.get(key));
+    }
+
+    private Map.Entry<Map<String, Object>, MarshalledValue<Map<String, Object>, MarshallingContext>> getValue(String id, Function<SessionAttributesKey, MarshalledValue<Map<String, Object>, MarshallingContext>> supplier) {
+        SessionAttributesKey key = new SessionAttributesKey(id);
+        MarshalledValue<Map<String, Object>, MarshallingContext> value = supplier.apply(key);
         if (value != null) {
             try {
                 Map<String, Object> attributes = this.marshaller.read(value);

clustering/web/infinispan/src/main/java/org/wildfly/clustering/web/infinispan/session/fine/FineImmutableSessionAttributes.java

@@ -21,8 +21,8 @@
  */
 package org.wildfly.clustering.web.infinispan.session.fine;
 
+import java.util.Map;
 import java.util.Set;
-import java.util.stream.Collectors;
 
 import org.infinispan.Cache;
 import org.wildfly.clustering.marshalling.jboss.InvalidSerializedFormException;
@@ -37,28 +37,30 @@
  */
 public class FineImmutableSessionAttributes<V> implements ImmutableSessionAttributes {
     private final String id;
+    private final Map<String, Integer> names;
     private final Cache<SessionAttributeKey, V> cache;
     private final Marshaller<Object, V, MarshallingContext> marshaller;
 
-    public FineImmutableSessionAttributes(String id, Cache<SessionAttributeKey, V> attributeCache, Marshaller<Object, V, MarshallingContext> marshaller) {
+    public FineImmutableSessionAttributes(String id, Map<String, Integer> names, Cache<SessionAttributeKey, V> attributeCache, Marshaller<Object, V, MarshallingContext> marshaller) {
         this.id = id;
+        this.names = names;
         this.cache = attributeCache;
         this.marshaller = marshaller;
     }
 
     @Override
     public Set<String> getAttributeNames() {
-        return this.cache.getAdvancedCache().getGroup(this.id).keySet().stream().filter((Object object) -> object instanceof SessionAttributeKey).map(key -> key.getAttribute()).collect(Collectors.toSet());
+        return this.names.keySet();
     }
 
     @Override
     public Object getAttribute(String name) {
-        SessionAttributeKey key = this.createKey(name);
-        return this.read(name, this.cache.get(key));
+        Integer attributeId = this.names.get(name);
+        return (attributeId != null) ? this.read(name, this.cache.get(this.createKey(attributeId))) : null;
     }
 
-    protected SessionAttributeKey createKey(String attribute) {
-        return new SessionAttributeKey(this.id, attribute);
+    protected SessionAttributeKey createKey(int attributeId) {
+        return new SessionAttributeKey(this.id, attributeId);
     }
 
     protected Object read(String name, V value) {

clustering/web/infinispan/src/main/java/org/wildfly/clustering/web/infinispan/session/fine/FineSessionAttributes.java

@@ -23,6 +23,8 @@
 
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.atomic.AtomicInteger;
 
 import org.infinispan.Cache;
 import org.infinispan.commons.marshall.NotSerializableException;
@@ -40,21 +42,30 @@
  * @author Paul Ferraro
  */
 public class FineSessionAttributes<V> extends FineImmutableSessionAttributes<V> implements SessionAttributes {
+    private final AtomicInteger sequence;
+    private final ConcurrentMap<String, Integer> names;
+    private final Mutator namesMutator;
     private final Cache<SessionAttributeKey, V> cache;
     private final Map<String, Mutator> mutations = new ConcurrentHashMap<>();
     private final Marshaller<Object, V, MarshallingContext> marshaller;
     private final CacheProperties properties;
 
-    public FineSessionAttributes(String id, Cache<SessionAttributeKey, V> cache, Marshaller<Object, V, MarshallingContext> marshaller, CacheProperties properties) {
-        super(id, cache, marshaller);
+    public FineSessionAttributes(String id, AtomicInteger sequence, ConcurrentMap<String, Integer> names, Mutator namesMutator, Cache<SessionAttributeKey, V> cache, Marshaller<Object, V, MarshallingContext> marshaller, CacheProperties properties) {
+        super(id, names, cache, marshaller);
+        this.sequence = sequence;
+        this.names = names;
+        this.namesMutator = namesMutator;
         this.cache = cache;
         this.marshaller = marshaller;
         this.properties = properties;
     }
 
     @Override
     public Object removeAttribute(String name) {
-        SessionAttributeKey key = this.createKey(name);
+        Integer attributeId = this.names.remove(name);
+        if (attributeId == null) return null;
+        this.namesMutator.mutate();
+        SessionAttributeKey key = this.createKey(attributeId);
         Object result = this.read(name, this.cache.getAdvancedCache().withFlags(Flag.FORCE_SYNCHRONOUS).remove(key));
         this.mutations.remove(name);
         return result;
@@ -68,16 +79,23 @@ public Object setAttribute(String name, Object attribute) {
         if (this.properties.isMarshalling() && !this.marshaller.getContext().isMarshallable(attribute)) {
             throw new IllegalArgumentException(new NotSerializableException(attribute.getClass().getName()));
         }
-        SessionAttributeKey key = this.createKey(name);
         V value = this.marshaller.write(attribute);
+        int currentId = this.sequence.get();
+        int attributeId = this.names.computeIfAbsent(name, key -> this.sequence.incrementAndGet());
+        if (attributeId > currentId) {
+            this.namesMutator.mutate();
+        }
+        SessionAttributeKey key = this.createKey(attributeId);
         Object result = this.read(name, this.cache.getAdvancedCache().withFlags(Flag.FORCE_SYNCHRONOUS).put(key, value));
         this.mutations.remove(name);
         return result;
     }
 
     @Override
     public Object getAttribute(String name) {
-        SessionAttributeKey key = this.createKey(name);
+        Integer attributeId = this.names.get(name);
+        if (attributeId == null) return null;
+        SessionAttributeKey key = this.createKey(attributeId);
         V value = this.cache.get(key);
         Object attribute = this.read(name, value);
         if (attribute != null) {
@@ -96,7 +114,7 @@ public Object getAttribute(String name) {
     @Override
     public void close() {
         if (!this.properties.isTransactional()) {
-            this.mutations.values().forEach(mutator -> mutator.mutate());
+            this.mutations.values().forEach(Mutator::mutate);
         }
         this.mutations.clear();
     }