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 Authored: Thu Sep 24 14:09:21 2015 -0700 Committer: Sylvain Lebresne 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 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 { 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 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;