Skip to content

Commit

Permalink
Internal byte[] reuse in StringSchemaKey
Browse files Browse the repository at this point in the history
Which should lower garbage churn during range results
as well as improve performance.
  • Loading branch information
tinwelint committed Mar 26, 2018
1 parent bcee440 commit 7336165
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 19 deletions.
Expand Up @@ -44,10 +44,7 @@ public StringSchemaKey newKey()
@Override @Override
public StringSchemaKey copyKey( StringSchemaKey key, StringSchemaKey into ) public StringSchemaKey copyKey( StringSchemaKey key, StringSchemaKey into )
{ {
// TODO when we have reuse of byte[] take that into consideration here too into.copyFrom( key );
into.bytes = key.bytes.clone();
into.setEntityId( key.getEntityId() );
into.setCompareId( key.getCompareId() );
return into; return into;
} }


Expand All @@ -61,21 +58,22 @@ public int keySize( StringSchemaKey key )
public void writeKey( PageCursor cursor, StringSchemaKey key ) public void writeKey( PageCursor cursor, StringSchemaKey key )
{ {
cursor.putLong( key.getEntityId() ); cursor.putLong( key.getEntityId() );
cursor.putBytes( key.bytes ); cursor.putBytes( key.bytes, 0, key.bytesLength );
} }


@Override @Override
public void readKey( PageCursor cursor, StringSchemaKey into, int keySize ) public void readKey( PageCursor cursor, StringSchemaKey into, int keySize )
{ {
// TODO consider reusing byte[] instances somehow
if ( keySize < ENTITY_ID_SIZE ) if ( keySize < ENTITY_ID_SIZE )
{ {
into.bytes = null; into.setEntityId( Long.MIN_VALUE );
into.setBytesLength( 0 );
return; return;
} }
into.setEntityId( cursor.getLong() ); into.setEntityId( cursor.getLong() );
into.bytes = new byte[keySize - ENTITY_ID_SIZE]; int bytesLength = keySize - ENTITY_ID_SIZE;
cursor.getBytes( into.bytes ); into.setBytesLength( bytesLength );
cursor.getBytes( into.bytes, 0, bytesLength );
} }


@Override @Override
Expand Down
Expand Up @@ -38,11 +38,16 @@ class StringSchemaKey extends NativeSchemaKey<StringSchemaKey>


private boolean ignoreLength; private boolean ignoreLength;


// UTF-8 bytes, grows on demand. Actual length is dictated by bytesLength field.
byte[] bytes; byte[] bytes;
int bytesLength;
// Set to true when the internal byte[] have been handed out to an UTF8Value, so that the next call to setBytesLength
// will be forced to allocate a new array. The byte[] isn't cleared with null since this key still logically contains those bytes.
private boolean bytesDereferenced;


int size() int size()
{ {
return ENTITY_ID_SIZE + bytes.length; return ENTITY_ID_SIZE + bytesLength;
} }


@Override @Override
Expand All @@ -66,7 +71,14 @@ void initialize( long entityId )
@Override @Override
public Value asValue() public Value asValue()
{ {
return bytes == null ? Values.NO_VALUE : Values.utf8Value( bytes ); if ( bytes == null )
{
return Values.NO_VALUE;
}

// Dereference our bytes so that we won't overwrite it on next read
bytesDereferenced = true;
return Values.utf8Value( bytes, 0, bytesLength );
} }


@Override @Override
Expand Down Expand Up @@ -127,19 +139,19 @@ int compareValueTo( StringSchemaKey other )
return 0; return 0;
} }


return byteArrayCompare( bytes, other.bytes, ignoreLength | other.ignoreLength ); return unsignedByteArrayCompare( bytes, bytesLength, other.bytes, other.bytesLength, ignoreLength | other.ignoreLength );
} }


private static int byteArrayCompare( byte[] a, byte[] b, boolean ignoreLength ) private static int unsignedByteArrayCompare( byte[] a, int aLength, byte[] b, int bLength, boolean ignoreLength )
{ {
assert a != null && b != null : "Null arrays not supported."; assert a != null && b != null : "Null arrays not supported.";


if ( a == b ) if ( a == b && aLength == bLength )
{ {
return 0; return 0;
} }


int length = Math.min( a.length, b.length ); int length = Math.min( aLength, bLength );
for ( int i = 0; i < length; i++ ) for ( int i = 0; i < length; i++ )
{ {
int compare = Short.compare( (short) (a[i] & 0xFF), (short) (b[i] & 0xFF) ); int compare = Short.compare( (short) (a[i] & 0xFF), (short) (b[i] & 0xFF) );
Expand All @@ -149,24 +161,57 @@ private static int byteArrayCompare( byte[] a, byte[] b, boolean ignoreLength )
} }
} }


return ignoreLength ? 0 : Integer.compare( a.length, b.length ); return ignoreLength ? 0 : Integer.compare( aLength, bLength );
} }


@Override @Override
public String toString() public String toString()
{ {
return format( "value=%s,entityId=%d,bytes=%s", asValue(), getEntityId(), Arrays.toString( bytes ) ); return format( "value=%s,entityId=%d,bytes=%s",
asValue(),
getEntityId(),
bytes == null ? "null" : Arrays.toString( Arrays.copyOf( bytes, bytesLength ) ) );
} }


@Override @Override
public void writeString( String value ) public void writeString( String value )
{ {
bytes = UTF8.encode( value ); bytes = UTF8.encode( value );
bytesLength = bytes.length;
bytesDereferenced = false;
} }


@Override @Override
public void writeString( char value ) public void writeString( char value )
{ {
writeString( String.valueOf( value ) ); writeString( String.valueOf( value ) );
} }

void copyFrom( StringSchemaKey key )
{
setBytesLength( key.bytesLength );
System.arraycopy( key.bytes, 0, bytes, 0, key.bytesLength );
setEntityId( key.getEntityId() );
setCompareId( key.getCompareId() );
}

/**
* Ensures that the internal byte[] is long enough, or longer than the given {@code length}.
* Also sets the internal {@code bytesLength} field to the given {@code length} so that interactions with the byte[]
* from this point on will use that for length, instead of the length of the byte[].
*
* @param length minimum length that the internal byte[] needs to be.
*/
void setBytesLength( int length )
{
if ( bytesDereferenced || bytes == null || bytes.length < length )
{
bytesDereferenced = false;

// allocate a bit more than required so that there's a higher chance that this byte[] instance
// can be used for more keys than just this one
bytes = new byte[length + length / 2];
}
bytesLength = length;
}
} }
Expand Up @@ -33,7 +33,7 @@
import org.neo4j.values.storable.Values; import org.neo4j.values.storable.Values;


import static java.util.Arrays.asList; import static java.util.Arrays.asList;

import static java.util.Arrays.copyOf;
import static org.neo4j.values.storable.StringsLibrary.STRINGS; import static org.neo4j.values.storable.StringsLibrary.STRINGS;
import static org.neo4j.values.storable.UTF8StringValue.codePointByteArrayCompare; import static org.neo4j.values.storable.UTF8StringValue.codePointByteArrayCompare;


Expand All @@ -53,7 +53,9 @@ IndexQuery rangeQuery( Object from, boolean fromInclusive, Object to, boolean to
@Override @Override
int compareIndexedPropertyValue( StringSchemaKey key1, StringSchemaKey key2 ) int compareIndexedPropertyValue( StringSchemaKey key1, StringSchemaKey key2 )
{ {
return codePointByteArrayCompare( key1.bytes, key2.bytes ); return codePointByteArrayCompare(
copyOf( key1.bytes, key1.bytesLength ),
copyOf( key2.bytes, key2.bytesLength ) );
} }


@Override @Override
Expand Down

0 comments on commit 7336165

Please sign in to comment.