Skip to content

Commit

Permalink
Revert "Do not store (unused) property changes data in tx state"
Browse files Browse the repository at this point in the history
This reverts commit c87d945.
  • Loading branch information
Andrei Koval committed Mar 20, 2018
1 parent 90072aa commit e2a5f5d
Show file tree
Hide file tree
Showing 3 changed files with 172 additions and 0 deletions.
@@ -0,0 +1,99 @@
/*
* Copyright (c) 2002-2018 "Neo Technology,"
* Network Engine for Objects in Lund AB [http://neotechnology.com]
*
* This file is part of Neo4j.
*
* Neo4j is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program 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 General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
package org.neo4j.kernel.impl.api.state;

import java.util.Map;

import org.neo4j.kernel.impl.util.VersionedHashMap;
import org.neo4j.kernel.impl.util.collection.CollectionsFactory;
import org.neo4j.kernel.impl.util.diffsets.PrimitiveLongDiffSets;
import org.neo4j.storageengine.api.txstate.PrimitiveLongReadableDiffSets;

/**
* Indexes entities by what property and value has been modified on them.
*/
public class PropertyChanges
{
private final CollectionsFactory collectionsFactory;

private VersionedHashMap<Integer, Map<Object,PrimitiveLongDiffSets>> changes;

public PropertyChanges( CollectionsFactory collectionsFactory )
{
this.collectionsFactory = collectionsFactory;
}

public PrimitiveLongReadableDiffSets changesForProperty( int propertyKeyId, Object value )
{
if ( changes != null )
{
Map<Object,PrimitiveLongDiffSets> keyChanges = changes.get( propertyKeyId );
if ( keyChanges != null )
{
PrimitiveLongDiffSets valueChanges = keyChanges.get( value );
if ( valueChanges != null )
{
return valueChanges;
}
}
}
return PrimitiveLongReadableDiffSets.EMPTY;
}

public void changeProperty( long entityId, int propertyKeyId, Object oldValue, Object newValue )
{
Map<Object, PrimitiveLongDiffSets> keyChanges = keyChanges( propertyKeyId );
valueChanges( newValue, keyChanges ).add( entityId );
valueChanges( oldValue, keyChanges ).remove( entityId );
}

public void addProperty( long entityId, int propertyKeyId, Object value )
{
valueChanges( value, keyChanges( propertyKeyId ) ).add( entityId );
}

public void removeProperty( long entityId, int propertyKeyId, Object oldValue )
{
valueChanges( oldValue, keyChanges( propertyKeyId ) ).remove( entityId );
}

private Map<Object, PrimitiveLongDiffSets> keyChanges( int propertyKeyId )
{
if ( changes == null )
{
changes = new VersionedHashMap<>();
}

return changes.computeIfAbsent( propertyKeyId, k -> new VersionedHashMap<>() );
}

private PrimitiveLongDiffSets valueChanges( Object newValue, Map<Object, PrimitiveLongDiffSets> keyChanges )
{
return keyChanges.computeIfAbsent( newValue, k -> collectionsFactory.newLongDiffSets() );
}

public void release()
{
for ( final Map<Object, PrimitiveLongDiffSets> map : changes.values() )
{
map.values().forEach( PrimitiveLongDiffSets::close );
}
}
}
Expand Up @@ -113,6 +113,8 @@ public class TxState implements TransactionState, RelationshipVisitor.Home
private DiffSets<SchemaIndexDescriptor> indexChanges;
private DiffSets<ConstraintDescriptor> constraintsChanges;

private PropertyChanges propertyChangesForNodes;

private RemovalsCountingDiffSets nodes;
private RemovalsCountingRelationshipsDiffSets relationships;

Expand Down Expand Up @@ -529,13 +531,15 @@ public void nodeDoAddProperty( long nodeId, int newPropertyKeyId, Value value )
{
NodeStateImpl nodeState = getOrCreateNodeState( nodeId );
nodeState.addProperty( newPropertyKeyId, value );
nodePropertyChanges().addProperty( nodeId, newPropertyKeyId, value );
dataChanged();
}

@Override
public void nodeDoChangeProperty( long nodeId, int propertyKeyId, Value replacedValue, Value newValue )
{
getOrCreateNodeState( nodeId ).changeProperty( propertyKeyId, newValue );
nodePropertyChanges().changeProperty( nodeId, propertyKeyId, replacedValue, newValue );
dataChanged();
}

Expand Down Expand Up @@ -572,6 +576,7 @@ public void graphDoReplaceProperty( int propertyKeyId, Value replacedValue, Valu
public void nodeDoRemoveProperty( long nodeId, int propertyKeyId, Value removedValue )
{
getOrCreateNodeState( nodeId ).removeProperty( propertyKeyId, removedValue );
nodePropertyChanges().removeProperty( nodeId, propertyKeyId, removedValue );
dataChanged();
}

Expand Down Expand Up @@ -1224,6 +1229,11 @@ private boolean hasNodeState( long nodeId )
return nodeStatesMap != null && nodeStatesMap.containsKey( nodeId );
}

private PropertyChanges nodePropertyChanges()
{
return propertyChangesForNodes == null ? propertyChangesForNodes = new PropertyChanges( collectionsFactory ) : propertyChangesForNodes;
}

@Override
public PrimitiveLongResourceIterator augmentNodesGetAll( PrimitiveLongIterator committed )
{
Expand Down Expand Up @@ -1277,6 +1287,10 @@ public void release()
{
relationshipStatesMap.close();
}
if ( propertyChangesForNodes != null )
{
propertyChangesForNodes.release();
}
if ( nodes != null && nodes.removedFromAdded != null )
{
nodes.removedFromAdded.close();
Expand Down
@@ -0,0 +1,59 @@
/*
* Copyright (c) 2002-2018 "Neo Technology,"
* Network Engine for Objects in Lund AB [http://neotechnology.com]
*
* This file is part of Neo4j.
*
* Neo4j is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program 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 General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
package org.neo4j.kernel.impl.api.state;

import org.hamcrest.Matcher;
import org.junit.Test;

import org.neo4j.collection.primitive.PrimitiveLongSet;
import org.neo4j.kernel.impl.util.collection.OnHeapCollectionsFactory;
import org.neo4j.kernel.impl.util.diffsets.PrimitiveLongDiffSets;
import org.neo4j.storageengine.api.txstate.PrimitiveLongReadableDiffSets;

import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.neo4j.collection.primitive.PrimitiveLongCollections.emptySet;
import static org.neo4j.collection.primitive.PrimitiveLongCollections.setOf;

public class PropertyChangesTest
{
@Test
public void shouldListChanges()
{
// Given
PropertyChanges changes = new PropertyChanges( OnHeapCollectionsFactory.INSTANCE );
changes.changeProperty( 1L, 2, "from", "to" );
changes.addProperty( 1L, 3, "from" );
changes.removeProperty( 2L, 2, "to" );

// When & Then
assertThat( changes.changesForProperty( 2, "to" ), isDiffSets( setOf( 1L ), setOf( 2L ) ) );

assertThat( changes.changesForProperty( 3, "from" ), isDiffSets( setOf( 1L ), emptySet() ) );

assertThat( changes.changesForProperty( 2, "from" ), isDiffSets( emptySet(), setOf( 1L ) ) );
}

@SuppressWarnings( "unchecked" )
private Matcher<? super PrimitiveLongReadableDiffSets> isDiffSets( PrimitiveLongSet added, PrimitiveLongSet removed )
{
return (Matcher) equalTo( new PrimitiveLongDiffSets( added, removed, OnHeapCollectionsFactory.INSTANCE ) );
}
}

0 comments on commit e2a5f5d

Please sign in to comment.