Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#10534 PropertyTree memory overuse #10535

Merged
merged 1 commit into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import java.util.SortedSet;
import java.util.TreeSet;

import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSortedSet;

import com.enonic.xp.annotation.PublicApi;
Expand Down Expand Up @@ -35,7 +36,7 @@ private PatternIndexConfigDocument( final Builder builder )
{
super( builder );
this.pathIndexConfigs = ImmutableSortedSet.copyOf( builder.pathIndexConfigs );
this.pathIndexConfigMap = builder.stringPathIndexConfigMap;
this.pathIndexConfigMap = ImmutableMap.copyOf( builder.stringPathIndexConfigMap );
this.defaultConfig = builder.defaultConfig;
this.allTextConfig = builder.allTextIndexConfig.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ private NodeType( final String name )

public static NodeType from( final String name )
{
return new NodeType( name );
return DEFAULT_NODE_COLLECTION.name.equals( name ) ? DEFAULT_NODE_COLLECTION : new NodeType( name );
}

public String getName()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,24 +11,31 @@
public class OrderExpressions
extends AbstractImmutableEntityList<OrderExpr>
{
private static final OrderExpressions EMPTY = new OrderExpressions( ImmutableList.of() );

private OrderExpressions( final ImmutableList<OrderExpr> list )
{
super( list );
}

public static OrderExpressions empty()
{
return new OrderExpressions( ImmutableList.of() );
return EMPTY;
}

public static OrderExpressions from( final OrderExpr... orderExprs )
{
return new OrderExpressions( ImmutableList.copyOf( orderExprs ) );
return fromInternal( ImmutableList.copyOf( orderExprs ) );
}

public static OrderExpressions from( final Collection<OrderExpr> orderExprs )
{
return new OrderExpressions( ImmutableList.copyOf( orderExprs ) );
return fromInternal( ImmutableList.copyOf( orderExprs ) );
}

private static OrderExpressions fromInternal( final ImmutableList<OrderExpr> set )
{
return set.isEmpty() ? EMPTY : new OrderExpressions( set );
}

public static Builder create()
Expand All @@ -48,7 +55,7 @@ public Builder add( final OrderExpr orderExpr )

public OrderExpressions build()
{
return new OrderExpressions( this.orderExprs.build() );
return fromInternal( orderExprs.build() );
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,28 +11,35 @@
public class BinaryReferences
extends AbstractImmutableEntitySet<BinaryReference>
{
private static final BinaryReferences EMPTY = new BinaryReferences( ImmutableSet.of() );

private BinaryReferences( final ImmutableSet<BinaryReference> set )
{
super( set );
}

public static BinaryReferences empty()
{
return new BinaryReferences( ImmutableSet.of() );
return EMPTY;
}

public static BinaryReferences from( final String... binaryReferences )
{
return new BinaryReferences( Stream.of( binaryReferences ).map( BinaryReference::from ).collect( ImmutableSet.toImmutableSet() ) );
return fromInternal( Stream.of( binaryReferences ).map( BinaryReference::from ).collect( ImmutableSet.toImmutableSet() ) );
}

public static BinaryReferences from( final BinaryReference... binaryReferences )
{
return new BinaryReferences( ImmutableSet.copyOf( binaryReferences ) );
return fromInternal( ImmutableSet.copyOf( binaryReferences ) );

Check warning on line 33 in modules/core/core-api/src/main/java/com/enonic/xp/util/BinaryReferences.java

View check run for this annotation

Codecov / codecov/patch

modules/core/core-api/src/main/java/com/enonic/xp/util/BinaryReferences.java#L33

Added line #L33 was not covered by tests
}

public static BinaryReferences from( final Iterable<BinaryReference> binaryReferences )
{
return new BinaryReferences( ImmutableSet.copyOf( binaryReferences ) );
return fromInternal( ImmutableSet.copyOf( binaryReferences ) );
}

private static BinaryReferences fromInternal( final ImmutableSet<BinaryReference> set )
{
return set.isEmpty() ? EMPTY : new BinaryReferences( set );
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.enonic.xp.repo.impl.node.dao;

import java.util.List;
import java.util.concurrent.ExecutionException;

import org.osgi.service.component.annotations.Activate;
Expand All @@ -18,11 +19,15 @@
import com.enonic.xp.blob.NodeVersionKey;
import com.enonic.xp.blob.Segment;
import com.enonic.xp.blob.SegmentLevel;
import com.enonic.xp.data.PropertyTree;
import com.enonic.xp.index.IndexConfigDocument;
import com.enonic.xp.node.NodeVersion;
import com.enonic.xp.repo.impl.InternalContext;
import com.enonic.xp.repo.impl.config.RepoConfiguration;
import com.enonic.xp.repo.impl.node.NodeConstants;
import com.enonic.xp.repo.impl.node.json.ImmutableNodeVersion;
import com.enonic.xp.repo.impl.node.json.ImmutableProperty;
import com.enonic.xp.repo.impl.node.json.ImmutableVersionData;
import com.enonic.xp.repo.impl.node.json.NodeVersionAccessControl;
import com.enonic.xp.repo.impl.node.json.NodeVersionJsonSerializer;
import com.enonic.xp.repository.RepositoryId;
Expand All @@ -34,7 +39,7 @@ public class NodeVersionServiceImpl
{
private final BlobStore blobStore;

private final Cache<BlobKey, NodeVersion> nodeDataCache;
private final Cache<BlobKey, ImmutableNodeVersion> nodeDataCache;

private final Cache<BlobKey, IndexConfigDocument> indexConfigCache;

Expand Down Expand Up @@ -78,16 +83,19 @@ public NodeVersion get( final NodeVersionKey nodeVersionKey, final InternalConte

try
{
final NodeVersion nodeVersion = nodeDataCache.get( nodeBlobKey, () -> {
final ImmutableNodeVersion immutableNodeVersion = nodeDataCache.get( nodeBlobKey, () -> {
final BlobRecord nodeBlobRecord = getBlobRecord( NodeConstants.NODE_SEGMENT_LEVEL, context.getRepositoryId(), nodeBlobKey );
return NodeVersionJsonSerializer.toNodeVersionData( nodeBlobRecord.getBytes() );

try (var is = nodeBlobRecord.getBytes().openBufferedStream())
{
return ImmutableVersionData.deserialize( is );
}
} );

final IndexConfigDocument indexConfigDocument = indexConfigCache.get( indexConfigBlobKey, () -> {
final BlobRecord indexConfigBlobRecord =
getBlobRecord( NodeConstants.INDEX_CONFIG_SEGMENT_LEVEL, context.getRepositoryId(), indexConfigBlobKey );
return NodeVersionJsonSerializer.toIndexConfigDocument( indexConfigBlobRecord.getBytes() );

} );

final NodeVersionAccessControl accessControl = accessControlCache.get( accessControlBlobKey, () -> {
Expand All @@ -96,8 +104,13 @@ public NodeVersion get( final NodeVersionKey nodeVersionKey, final InternalConte
return NodeVersionJsonSerializer.toNodeVersionAccessControl( accessControlBlobRecord.getBytes() );
} );


return NodeVersion.create( nodeVersion )
return NodeVersion.create()
.id( immutableNodeVersion.id )
.nodeType( immutableNodeVersion.nodeType )
.data( toPropertyTree( immutableNodeVersion.data ) )
.childOrder( immutableNodeVersion.childOrder )
.manualOrderValue( immutableNodeVersion.manualOrderValue )
.attachedBinaries( immutableNodeVersion.attachedBinaries )
.indexConfigDocument( indexConfigDocument )
.permissions( accessControl.getPermissions() )
.build();
Expand All @@ -115,6 +128,13 @@ public NodeVersion get( final NodeVersionKey nodeVersionKey, final InternalConte
}
}

static PropertyTree toPropertyTree( final List<ImmutableProperty> data )
{
final PropertyTree result = new PropertyTree();
ImmutableProperty.addToSet( result.getRoot(), data );
return result;
}

private BlobRecord getBlobRecord( SegmentLevel segmentLevel, RepositoryId repositoryId, BlobKey blobKey )
{
final Segment nodeSegment = RepositorySegmentUtils.toSegment( repositoryId, segmentLevel );
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package com.enonic.xp.repo.impl.node.json;

import java.util.List;

import com.enonic.xp.index.ChildOrder;
import com.enonic.xp.node.AttachedBinaries;
import com.enonic.xp.node.NodeId;
import com.enonic.xp.node.NodeType;

public final class ImmutableNodeVersion
{
public final NodeId id;

public final NodeType nodeType;

public final List<ImmutableProperty> data;

public final ChildOrder childOrder;

public final Long manualOrderValue;

public final AttachedBinaries attachedBinaries;

public ImmutableNodeVersion( final NodeId id, final NodeType nodeType, final List<ImmutableProperty> data,
final ChildOrder childOrder,
final Long manualOrderValue,
final AttachedBinaries attachedBinaries )
{
this.id = id;
this.nodeType = nodeType;
this.data = List.copyOf( data );
this.childOrder = childOrder;
this.manualOrderValue = manualOrderValue;
this.attachedBinaries = attachedBinaries;
}
}