cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From slebre...@apache.org
Subject cassandra git commit: Minor optimizations
Date Fri, 02 Oct 2015 13:51:39 GMT
Repository: cassandra
Updated Branches:
  refs/heads/trunk 2216d3924 -> 2d991a7e4


Minor optimizations

patch by slebresne; reviewed by blambov for CASSANDRA-10410

The patch includes the 3 minor optimizations desribed below:
1) Faster copy in CompositeType.build(). We call CompositeType.build()
   relatively often when a table has a composite partition key. When
   copying source buffers into the composite result in that method, we
   can use our existing ByteBuffer.arrayCopy() method which is supposed
   to be faster since it uses FastByteOperations and saves the
   duplication of the buffer to copy
2) Faster UUID->ByteBuffer. We convert UUID to their ByteBuffer
    representation in a few places and do that by converting the UUID to
    a byte[] and then wrapping it. But our convertion of UUID to byte[]
    writes the 2 long the UUID is composed of byte by byte, while for a
    ByteBuffer we can easily just "put" both long. So the patch
    introduces a specific method to do that. That's arguably cleaner
    anyway.
3) Memoize hash value for DataResource. Every request does a hash map
   lookup on a DataResource (for validating access rights) which imply
   calling DataResource.hashCode(). Currently that uses
   Objects.hashCode(), which is a varargs methods and allocate an array
   (which, according to MissionControl seems to not be stack allocated).
   The patch does 2 things: it computes the hash only once in the ctor
   and it stores the DataResource for each table in the CFMetaData and
   uses that when checking access permissions for SelectStatement and
   ModificationStatement (the 2 statements where performance matters).
   The 2nd point also saves some unecessary test that the table exists
   when we actually know it does.


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/2d991a7e
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/2d991a7e
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/2d991a7e

Branch: refs/heads/trunk
Commit: 2d991a7e4cc73b522f2c8adf14b5ff37a7947427
Parents: 2216d39
Author: Sylvain Lebresne <sylvain@datastax.com>
Authored: Thu Sep 24 14:09:21 2015 -0700
Committer: Sylvain Lebresne <sylvain@datastax.com>
Committed: Fri Oct 2 15:51:07 2015 +0200

----------------------------------------------------------------------
 .../org/apache/cassandra/auth/DataResource.java | 29 +++++++-------------
 .../org/apache/cassandra/config/CFMetaData.java |  4 +++
 .../cassandra/cql3/functions/TimeFcts.java      |  4 +--
 .../cql3/statements/ModificationStatement.java  |  8 +++---
 .../cql3/statements/SelectStatement.java        |  4 +--
 .../cassandra/db/marshal/CompositeType.java     |  4 ++-
 .../apache/cassandra/db/marshal/UUIDType.java   |  2 +-
 .../cassandra/serializers/UUIDSerializer.java   |  2 +-
 .../apache/cassandra/service/ClientState.java   |  7 +++++
 .../org/apache/cassandra/utils/UUIDGen.java     |  9 ++++++
 .../org/apache/cassandra/utils/UUIDTests.java   | 10 ++++++-
 11 files changed, 52 insertions(+), 31 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/2d991a7e/src/java/org/apache/cassandra/auth/DataResource.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/auth/DataResource.java b/src/java/org/apache/cassandra/auth/DataResource.java
index f64ed93..0aa24db 100644
--- a/src/java/org/apache/cassandra/auth/DataResource.java
+++ b/src/java/org/apache/cassandra/auth/DataResource.java
@@ -54,31 +54,22 @@ public class DataResource implements IResource
                                                                                         
   Permission.MODIFY,
                                                                                         
   Permission.AUTHORIZE);
     private static final String ROOT_NAME = "data";
-    private static final DataResource ROOT_RESOURCE = new DataResource();
+    private static final DataResource ROOT_RESOURCE = new DataResource(Level.ROOT, null,
null);
 
     private final Level level;
     private final String keyspace;
     private final String table;
 
-    private DataResource()
-    {
-        level = Level.ROOT;
-        keyspace = null;
-        table = null;
-    }
+    // memoized hashcode since DataRessource is immutable and used in hashmaps often
+    private final transient int hash;
 
-    private DataResource(String keyspace)
+    private DataResource(Level level, String keyspace, String table)
     {
-        level = Level.KEYSPACE;
-        this.keyspace = keyspace;
-        table = null;
-    }
-
-    private DataResource(String keyspace, String table)
-    {
-        level = Level.TABLE;
+        this.level = level;
         this.keyspace = keyspace;
         this.table = table;
+
+        this.hash = Objects.hashCode(level, keyspace, table);
     }
 
     /**
@@ -97,7 +88,7 @@ public class DataResource implements IResource
      */
     public static DataResource keyspace(String keyspace)
     {
-        return new DataResource(keyspace);
+        return new DataResource(Level.KEYSPACE, keyspace, null);
     }
 
     /**
@@ -109,7 +100,7 @@ public class DataResource implements IResource
      */
     public static DataResource table(String keyspace, String table)
     {
-        return new DataResource(keyspace, table);
+        return new DataResource(Level.TABLE, keyspace, table);
     }
 
     /**
@@ -272,6 +263,6 @@ public class DataResource implements IResource
     @Override
     public int hashCode()
     {
-        return Objects.hashCode(level, keyspace, table);
+        return hash;
     }
 }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/2d991a7e/src/java/org/apache/cassandra/config/CFMetaData.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/config/CFMetaData.java b/src/java/org/apache/cassandra/config/CFMetaData.java
index 00ca704..5db1985 100644
--- a/src/java/org/apache/cassandra/config/CFMetaData.java
+++ b/src/java/org/apache/cassandra/config/CFMetaData.java
@@ -38,6 +38,7 @@ import org.apache.commons.lang3.builder.ToStringBuilder;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import org.apache.cassandra.auth.DataResource;
 import org.apache.cassandra.cql3.ColumnIdentifier;
 import org.apache.cassandra.cql3.QueryProcessor;
 import org.apache.cassandra.cql3.statements.CFStatement;
@@ -118,6 +119,8 @@ public final class CFMetaData
     // for those tables in practice).
     private volatile ColumnDefinition compactValueColumn;
 
+    public final DataResource resource;
+
     /*
      * All of these methods will go away once CFMetaData becomes completely immutable.
      */
@@ -288,6 +291,7 @@ public final class CFMetaData
         this.partitionColumns = partitionColumns;
 
         this.serializers = new Serializers(this);
+        this.resource = DataResource.table(ksName, cfName);
         rebuild();
     }
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/2d991a7e/src/java/org/apache/cassandra/cql3/functions/TimeFcts.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/functions/TimeFcts.java b/src/java/org/apache/cassandra/cql3/functions/TimeFcts.java
index 93d6d3b..1b87261 100644
--- a/src/java/org/apache/cassandra/cql3/functions/TimeFcts.java
+++ b/src/java/org/apache/cassandra/cql3/functions/TimeFcts.java
@@ -67,7 +67,7 @@ public abstract class TimeFcts
             if (bb == null)
                 return null;
 
-            return ByteBuffer.wrap(UUIDGen.decompose(UUIDGen.minTimeUUID(TimestampType.instance.compose(bb).getTime())));
+            return UUIDGen.toByteBuffer(UUIDGen.minTimeUUID(TimestampType.instance.compose(bb).getTime()));
         }
     };
 
@@ -79,7 +79,7 @@ public abstract class TimeFcts
             if (bb == null)
                 return null;
 
-            return ByteBuffer.wrap(UUIDGen.decompose(UUIDGen.maxTimeUUID(TimestampType.instance.compose(bb).getTime())));
+            return UUIDGen.toByteBuffer(UUIDGen.maxTimeUUID(TimestampType.instance.compose(bb).getTime()));
         }
     };
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/2d991a7e/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java
index 23a26d0..7eefd8e 100644
--- a/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/ModificationStatement.java
@@ -193,21 +193,21 @@ public abstract class ModificationStatement implements CQLStatement
 
     public void checkAccess(ClientState state) throws InvalidRequestException, UnauthorizedException
     {
-        state.hasColumnFamilyAccess(keyspace(), columnFamily(), Permission.MODIFY);
+        state.hasColumnFamilyAccess(cfm, Permission.MODIFY);
 
         // CAS updates can be used to simulate a SELECT query, so should require Permission.SELECT
as well.
         if (hasConditions())
-            state.hasColumnFamilyAccess(keyspace(), columnFamily(), Permission.SELECT);
+            state.hasColumnFamilyAccess(cfm, Permission.SELECT);
 
         // MV updates need to get the current state from the table, and might update the
views
         // Require Permission.SELECT on the base table, and Permission.MODIFY on the views
         Iterator<ViewDefinition> views = View.findAll(keyspace(), columnFamily()).iterator();
         if (views.hasNext())
         {
-            state.hasColumnFamilyAccess(keyspace(), columnFamily(), Permission.SELECT);
+            state.hasColumnFamilyAccess(cfm, Permission.SELECT);
             do
             {
-                state.hasColumnFamilyAccess(keyspace(), views.next().viewName, Permission.MODIFY);
+                state.hasColumnFamilyAccess(views.next().metadata, Permission.MODIFY);
             } while (views.hasNext());
         }
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/2d991a7e/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
index 7848556..21bb257 100644
--- a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
+++ b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java
@@ -178,11 +178,11 @@ public class SelectStatement implements CQLStatement
         {
             CFMetaData baseTable = View.findBaseTable(keyspace(), columnFamily());
             if (baseTable != null)
-                state.hasColumnFamilyAccess(keyspace(), baseTable.cfName, Permission.SELECT);
+                state.hasColumnFamilyAccess(baseTable, Permission.SELECT);
         }
         else
         {
-            state.hasColumnFamilyAccess(keyspace(), columnFamily(), Permission.SELECT);
+            state.hasColumnFamilyAccess(cfm, Permission.SELECT);
         }
 
         for (Function function : getFunctions())

http://git-wip-us.apache.org/repos/asf/cassandra/blob/2d991a7e/src/java/org/apache/cassandra/db/marshal/CompositeType.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/marshal/CompositeType.java b/src/java/org/apache/cassandra/db/marshal/CompositeType.java
index 842cbbf..9bd23dd 100644
--- a/src/java/org/apache/cassandra/db/marshal/CompositeType.java
+++ b/src/java/org/apache/cassandra/db/marshal/CompositeType.java
@@ -391,7 +391,9 @@ public class CompositeType extends AbstractCompositeType
         for (ByteBuffer bb : buffers)
         {
             ByteBufferUtil.writeShortLength(out, bb.remaining());
-            out.put(bb.duplicate());
+            int toCopy = bb.remaining();
+            ByteBufferUtil.arrayCopy(bb, bb.position(), out, out.position(), toCopy);
+            out.position(out.position() + toCopy);
             out.put((byte) 0);
         }
         out.flip();

http://git-wip-us.apache.org/repos/asf/cassandra/blob/2d991a7e/src/java/org/apache/cassandra/db/marshal/UUIDType.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/marshal/UUIDType.java b/src/java/org/apache/cassandra/db/marshal/UUIDType.java
index acaf27c..9722a52 100644
--- a/src/java/org/apache/cassandra/db/marshal/UUIDType.java
+++ b/src/java/org/apache/cassandra/db/marshal/UUIDType.java
@@ -140,7 +140,7 @@ public class UUIDType extends AbstractType<UUID>
         {
             try
             {
-                return ByteBuffer.wrap(UUIDGen.decompose(UUID.fromString(source)));
+                return UUIDGen.toByteBuffer(UUID.fromString(source));
             }
             catch (IllegalArgumentException e)
             {

http://git-wip-us.apache.org/repos/asf/cassandra/blob/2d991a7e/src/java/org/apache/cassandra/serializers/UUIDSerializer.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/serializers/UUIDSerializer.java b/src/java/org/apache/cassandra/serializers/UUIDSerializer.java
index f8e2582..4501f34 100644
--- a/src/java/org/apache/cassandra/serializers/UUIDSerializer.java
+++ b/src/java/org/apache/cassandra/serializers/UUIDSerializer.java
@@ -34,7 +34,7 @@ public class UUIDSerializer implements TypeSerializer<UUID>
 
     public ByteBuffer serialize(UUID value)
     {
-        return value == null ? ByteBufferUtil.EMPTY_BYTE_BUFFER : ByteBuffer.wrap(UUIDGen.decompose(value));
+        return value == null ? ByteBufferUtil.EMPTY_BYTE_BUFFER : UUIDGen.toByteBuffer(value);
     }
 
     public void validate(ByteBuffer bytes) throws MarshalException

http://git-wip-us.apache.org/repos/asf/cassandra/blob/2d991a7e/src/java/org/apache/cassandra/service/ClientState.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/service/ClientState.java b/src/java/org/apache/cassandra/service/ClientState.java
index f2ed984..d576ac3 100644
--- a/src/java/org/apache/cassandra/service/ClientState.java
+++ b/src/java/org/apache/cassandra/service/ClientState.java
@@ -29,6 +29,7 @@ import org.slf4j.LoggerFactory;
 
 import org.apache.cassandra.auth.*;
 import org.apache.cassandra.config.Config;
+import org.apache.cassandra.config.CFMetaData;
 import org.apache.cassandra.config.DatabaseDescriptor;
 import org.apache.cassandra.config.Schema;
 import org.apache.cassandra.cql3.QueryHandler;
@@ -249,6 +250,12 @@ public class ClientState
         hasAccess(keyspace, perm, DataResource.table(keyspace, columnFamily));
     }
 
+    public void hasColumnFamilyAccess(CFMetaData cfm, Permission perm)
+    throws UnauthorizedException, InvalidRequestException
+    {
+        hasAccess(cfm.ksName, perm, cfm.resource);
+    }
+
     private void hasAccess(String keyspace, Permission perm, DataResource resource)
     throws UnauthorizedException, InvalidRequestException
     {

http://git-wip-us.apache.org/repos/asf/cassandra/blob/2d991a7e/src/java/org/apache/cassandra/utils/UUIDGen.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/utils/UUIDGen.java b/src/java/org/apache/cassandra/utils/UUIDGen.java
index a777a50..f046279 100644
--- a/src/java/org/apache/cassandra/utils/UUIDGen.java
+++ b/src/java/org/apache/cassandra/utils/UUIDGen.java
@@ -106,6 +106,15 @@ public class UUIDGen
         return new UUID(raw.getLong(raw.position()), raw.getLong(raw.position() + 8));
     }
 
+    public static ByteBuffer toByteBuffer(UUID uuid)
+    {
+        ByteBuffer buffer = ByteBuffer.allocate(16);
+        buffer.putLong(uuid.getMostSignificantBits());
+        buffer.putLong(uuid.getLeastSignificantBits());
+        buffer.flip();
+        return buffer;
+    }
+
     /** decomposes a uuid into raw bytes. */
     public static byte[] decompose(UUID uuid)
     {

http://git-wip-us.apache.org/repos/asf/cassandra/blob/2d991a7e/test/unit/org/apache/cassandra/utils/UUIDTests.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/utils/UUIDTests.java b/test/unit/org/apache/cassandra/utils/UUIDTests.java
index 83e421a..ebe5fd1 100644
--- a/test/unit/org/apache/cassandra/utils/UUIDTests.java
+++ b/test/unit/org/apache/cassandra/utils/UUIDTests.java
@@ -48,7 +48,6 @@ public class UUIDTests
         assert one.timestamp() < two.timestamp();
     }
 
-
     @Test
     public void testDecomposeAndRaw()
     {
@@ -59,6 +58,15 @@ public class UUIDTests
     }
 
     @Test
+    public void testToFromByteBuffer()
+    {
+        UUID a = UUIDGen.getTimeUUID();
+        ByteBuffer bb = UUIDGen.toByteBuffer(a);
+        UUID b = UUIDGen.getUUID(bb);
+        assert a.equals(b);
+    }
+
+    @Test
     public void testTimeUUIDType()
     {
         TimeUUIDType comp = TimeUUIDType.instance;


Mime
View raw message