jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From tri...@apache.org
Subject svn commit: r1538870 - /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/
Date Tue, 05 Nov 2013 05:44:53 GMT
Author: tripod
Date: Tue Nov  5 05:44:52 2013
New Revision: 1538870

URL: http://svn.apache.org/r1538870
Log:
OAK-1138 Implement global per principal permission entry cache (wip)

- make EntryIterator even more lazy (avoid seekNext() in next())
- only cache 'everyone's entries globally

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/AbstractEntryIterator.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryCache.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProvider.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImpl.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/AbstractEntryIterator.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/AbstractEntryIterator.java?rev=1538870&r1=1538869&r2=1538870&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/AbstractEntryIterator.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/AbstractEntryIterator.java
Tue Nov  5 05:44:52 2013
@@ -27,38 +27,39 @@ import com.google.common.collect.Iterato
  */
 abstract class AbstractEntryIterator implements Iterator<PermissionEntry> {
 
-    // the ordered permission entries at a given path in the hierarchy
-    protected Iterator<PermissionEntry> nextEntries;
-
     // the next permission entry
     protected PermissionEntry next;
 
+    // indicate that seekNext was already invoked
+    private boolean sought;
+
     @Override
     public boolean hasNext() {
-        if (next == null && nextEntries == null) {
-            // lazy initialization
-            nextEntries = Iterators.emptyIterator();
+        if (!sought) {
             seekNext();
+            sought = true;
         }
         return next != null;
     }
 
     @Override
     public PermissionEntry next() {
+        if (!sought) {
+            seekNext();
+            sought = true;
+        }
         if (next == null) {
             throw new NoSuchElementException();
         }
-
-        PermissionEntry pe = next;
-        seekNext();
-        return pe;
+        sought = false;
+        return next;
     }
 
+
     @Override
     public void remove() {
         throw new UnsupportedOperationException();
     }
 
-    @CheckForNull
     protected abstract void seekNext();
 }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java?rev=1538870&r1=1538869&r2=1538870&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java
Tue Nov  5 05:44:52 2013
@@ -520,7 +520,11 @@ final class CompiledPermissionImpl imple
 
     private static final class LazyIterator extends AbstractEntryIterator {
 
-        private boolean isUser;
+        private final boolean isUser;
+
+        // the ordered permission entries at a given path in the hierarchy
+        private Iterator<PermissionEntry> nextEntries = Iterators.emptyIterator();
+
         private TreePermissionImpl tp;
 
         private LazyIterator(@Nonnull TreePermissionImpl tp, boolean isUser) {
@@ -528,9 +532,9 @@ final class CompiledPermissionImpl imple
             this.isUser = isUser;
         }
 
-        @CheckForNull
+        @Override
         protected void seekNext() {
-            for (next = null; next == null; ) {
+            for (next = null; next == null;) {
                 if (nextEntries.hasNext()) {
                     next = nextEntries.next();
                 } else {

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryCache.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryCache.java?rev=1538870&r1=1538869&r2=1538870&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryCache.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryCache.java
Tue Nov  5 05:44:52 2013
@@ -16,18 +16,31 @@
  */
 package org.apache.jackrabbit.oak.security.authorization.permission;
 
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
+import java.util.TreeSet;
 import java.util.concurrent.ConcurrentHashMap;
 
+import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
+
 /**
- * <code>PermissionEntryCache</code> caches the permission entries of principals.
The cache is held globally and contains
+ * {@code PermissionEntryCache} caches the permission entries of principals. The cache is
held globally and contains
  * a version of the principal permission entries of the session that read them last. each
session gets a lazy copy of
  * the cache and needs to verify if each cached principal permission set still reflects the
state that the session sees.
  * every newly loaded principal permission set can be pushed down to the base cache if it
does not exist there yet, or
  * if it's newer.
+ *
+ * Todo:
+ * - currently only the entries of 'everyone' are globally cached. this should be improved
to dynamically cache those
+ *   principals that are used often
+ * - report cache usage metrics
+ * - limit size of local caches based on ppe sizes. the current implementation loads all
ppes. this can get a memory
+ *   problem, as well as a performance problem for principals with many entries. principals
with many entries must
+ *   fallback to the direct store.load() methods when providing the entries. if those principals
with many entries
+ *   are used often, they might get elected to live in the global cache; memory permitting.
  */
 public class PermissionEntryCache {
 
@@ -66,19 +79,45 @@ public class PermissionEntryCache {
                 }
             }
 
-            // check if base cache has the entries
-            PrincipalPermissionEntries baseppe = base.get(principalName);
-            if (baseppe == null || ppe.getTimestamp() > baseppe.getTimestamp()) {
-                base.put(principalName, ppe);
+            // currently we only cache 'everyones' entries. but the cache should dynamically
cache the principals
+            // that are used often.
+            if (EveryonePrincipal.NAME.equals(principalName)) {
+                // check if base cache has the entries
+                PrincipalPermissionEntries baseppe = base.get(principalName);
+                if (baseppe == null || ppe.getTimestamp() > baseppe.getTimestamp()) {
+                    base.put(principalName, ppe);
+                }
             }
             return ppe;
         }
 
+        public void load(PermissionStore store, Map<String, Collection<PermissionEntry>>
pathEntryMap, String principalName) {
+            // todo: conditionally load entries if too many
+            PrincipalPermissionEntries ppe = getEntries(store, principalName);
+            for (Map.Entry<String, Collection<PermissionEntry>> e: ppe.getEntries().entrySet())
{
+                Collection<PermissionEntry> pathEntries = pathEntryMap.get(e.getKey());
+                if (pathEntries == null) {
+                    pathEntries = new TreeSet<PermissionEntry>(e.getValue());
+                    pathEntryMap.put(e.getKey(), pathEntries);
+                } else {
+                    pathEntries.addAll(e.getValue());
+                }
+            }
+        }
+
+        public void load(PermissionStore store, Collection<PermissionEntry> ret, String
principalName, String path) {
+            // todo: conditionally load entries if too many
+            PrincipalPermissionEntries ppe = getEntries(store, principalName);
+            ret.addAll(ppe.getEntries(path));
+        }
+
         public boolean hasEntries(PermissionStore store, String principalName) {
+            // todo: conditionally load entries if too many
             return getNumEntries(store, principalName) > 0;
         }
 
         public long getNumEntries(PermissionStore store, String principalName) {
+            // todo: conditionally load entries if too many
             return getEntries(store, principalName).getEntries().size();
         }
 

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProvider.java?rev=1538870&r1=1538869&r2=1538870&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProvider.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProvider.java
Tue Nov  5 05:44:52 2013
@@ -24,7 +24,7 @@ import javax.annotation.Nonnull;
 import org.apache.jackrabbit.oak.api.Tree;
 
 /**
- * <code>PermissionEntryProvider</code> provides permission entries for a given
set of principals.
+ * {@code PermissionEntryProvider} provides permission entries for a given set of principals.
  * It may internally hold a cache to improve performance and usually operates on the permission
store.
  */
 public interface PermissionEntryProvider {

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImpl.java?rev=1538870&r1=1538869&r2=1538870&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImpl.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntryProviderImpl.java
Tue Nov  5 05:44:52 2013
@@ -35,7 +35,7 @@ import com.google.common.base.Strings;
 import com.google.common.collect.Iterators;
 
 /**
- * <code>PermissionEntryProviderImpl</code>...
+ * {@code PermissionEntryProviderImpl} ...
  */
 public class PermissionEntryProviderImpl implements PermissionEntryProvider {
 
@@ -75,9 +75,15 @@ public class PermissionEntryProviderImpl
                 }
             }
         }
-        this.pathEntryMap = cnt < MAX_SIZE
-                ? createMap(cache, store, existingNames)
-                : null;
+        if (cnt < MAX_SIZE) {
+            // cache all entries of all principals
+            pathEntryMap = new HashMap<String, Collection<PermissionEntry>>();
+            for (String name: principalNames) {
+                cache.load(store, pathEntryMap, name);
+            }
+        } else {
+            pathEntryMap = null;
+        }
     }
 
     public void flush() {
@@ -122,35 +128,18 @@ public class PermissionEntryProviderImpl
     private Collection<PermissionEntry> loadEntries(@Nonnull String path) {
         Collection<PermissionEntry> ret = new TreeSet<PermissionEntry>();
         for (String name: existingNames) {
-            PrincipalPermissionEntries ppe = cache.getEntries(store, name);
-            ret.addAll(ppe.getEntries(path));
+            cache.load(store, ret, name, path);
         }
         return ret;
     }
 
-    private static Map<String, Collection<PermissionEntry>> createMap(@Nonnull
PermissionEntryCache.Local cache,
-                                                                      @Nonnull PermissionStore
store,
-                                                                      @Nonnull Set<String>
principalNames) {
-        Map<String, Collection<PermissionEntry>> pathEntryMap = new HashMap<String,
Collection<PermissionEntry>>();
-        for (String name: principalNames) {
-            PrincipalPermissionEntries ppe = cache.getEntries(store, name);
-            for (Map.Entry<String, Collection<PermissionEntry>> e: ppe.getEntries().entrySet())
{
-                Collection<PermissionEntry> pathEntries = pathEntryMap.get(e.getKey());
-                if (pathEntries == null) {
-                    pathEntries = new TreeSet<PermissionEntry>(e.getValue());
-                    pathEntryMap.put(e.getKey(), pathEntries);
-                } else {
-                    pathEntries.addAll(e.getValue());
-                }
-            }
-        }
-        return pathEntryMap;
-    }
-
     private final class EntryIterator extends AbstractEntryIterator {
 
         private final EntryPredicate predicate;
 
+        // the ordered permission entries at a given path in the hierarchy
+        private Iterator<PermissionEntry> nextEntries = Iterators.emptyIterator();
+
         // the next oak path for which to retrieve permission entries
         private String path;
 
@@ -159,9 +148,9 @@ public class PermissionEntryProviderImpl
             this.path = Strings.nullToEmpty(predicate.getPath());
         }
 
-        @CheckForNull
+        @Override
         protected void seekNext() {
-            for (next = null; next == null; ) {
+            for (next = null; next == null;) {
                 if (nextEntries.hasNext()) {
                     PermissionEntry pe = nextEntries.next();
                     if (predicate.apply(pe)) {



Mime
View raw message