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

Collection optimize for OLTP algorithm and HugeElement #1409

Merged
merged 38 commits into from
Jun 9, 2021

Conversation

zhoney
Copy link
Contributor

@zhoney zhoney commented Mar 31, 2021

No description provided.

@@ -84,6 +84,7 @@ public String get(@Context GraphManager manager,
HugeTraverser.PathSet paths = traverser.paths(sourceId, dir, targetId,
dir, edgeLabel, depth,
degree, capacity, limit);
return manager.serializer(g).writePaths("crosspoints", paths, true);
return manager.serializer(g).writePaths("crosspoints",
paths.paths(),true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space

@@ -225,6 +225,12 @@ public long asLong() {
return 0L;
}

@Override
public int asInt() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't add this method, just force convert it

HugeGraph graph = ownerVertex.graph();
VertexLabel srcLabel = graph.vertexLabelOrNone(edgeLabel.sourceLabel());
VertexLabel tgtLabel = graph.vertexLabelOrNone(edgeLabel.targetLabel());

VertexLabel otherVertexLabel;
if (isOutEdge) {
ownerVertex.correctVertexLabel(srcLabel);
// ownerVertex.correctVertexLabel(srcLabel);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep the origin

otherVertexLabel = tgtLabel;
} else {
ownerVertex.correctVertexLabel(tgtLabel);
// ownerVertex.correctVertexLabel(tgtLabel);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

@@ -98,7 +100,7 @@ protected void updateToDefaultValueIfNone() {
this.defaultValueUpdated = true;
// Set default value if needed
for (Id pkeyId : this.schemaLabel().properties()) {
if (this.properties.containsKey(pkeyId)) {
if (this.properties.containsKey(pkeyId.asInt())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a static method numberFromId(id)

@@ -55,7 +55,7 @@ public void traverseOneLayer(Map<Id, List<Node>> vertices,

@Override
public Set<Path> newPathSet() {
return new HugeTraverser.PathSet();
return new HugeTraverser.PathSet().paths();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return HugeTraverser.PathSet()?

case 1:
return JCF;
case 2:
return EC;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add case 3 for FastUtil


import it.unimi.dsi.fastutil.objects.ObjectOpenHashSet;

public class IdSet extends HashSet<Id> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set final and implement Set<Id>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

implement Set need to implements toArray(), containsAll(), retainAll()... methods. which will be same as implemented in AbstractSet and HashSet. extends HashSet can reduce these duplicated work


private static class EcLongIterator implements Iterator<Long> {

private MutableLongIterator iterator;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set final

ExtendableIterator<Id> iterator = new ExtendableIterator<>();
iterator.extend(this.nonNumberIds.iterator());
EcLongIterator iter = new EcLongIterator(this.numberIds.longIterator());
iterator.extend(new MapperIterator<>(iter, IdGenerator::of));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let EcLongIterator return Id

@zhoney zhoney force-pushed the collection-optimize branch from 97d3f65 to cdc64f4 Compare April 7, 2021 13:11
@@ -50,6 +45,11 @@
import com.codahale.metrics.annotation.Timed;
import com.fasterxml.jackson.annotation.JsonProperty;

import static com.baidu.hugegraph.traversal.algorithm.HugeTraverser.DEFAULT_CAPACITY;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep origin place

@@ -47,6 +44,9 @@
import com.baidu.hugegraph.util.Log;
import com.codahale.metrics.annotation.Timed;

import static com.baidu.hugegraph.traversal.algorithm.HugeTraverser.DEFAULT_DEGREE;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

import javax.ws.rs.PathParam;
import javax.ws.rs.Produces;
import javax.ws.rs.QueryParam;
import javax.ws.rs.*;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't use *

private int accessed;

public ArrayPathsRecord(Id sourceV, Id targetV) {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove empty line

}
}

public static void main(String[] args) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move to unit test

return (Id) this.idMapping.code2Object(code);
}

public static class Int2IntsMap {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move out as an independent file

private static final int CHUNK_SIZE = 10;

private IntIntHashMap offsetMap;
private int[] intsTable;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add some comments

private IntIntHashMap offsetMap;
private int[] intsTable;

private int nextBlock;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

intsTableAlloc

this.intsTable[position] = this.nextBlock;

this.intsTable[this.nextBlock] = value;
this.intsTable[positionIndex] = this.nextBlock + 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why +1, prefer define const


} else {
intsTable[position] = value;
this.intsTable[positionIndex]++;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++this.intsTable[firstBlockPointer + SIZE_OFFSET]

// Update next block
this.nextBlock += CHUNK_SIZE;

} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

swap if-statement and the else code block

this.offsetMap.put(key, this.nextBlock);

// Init first chunk
this.intsTable[this.nextBlock] = this.nextBlock + 3;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

define const var for all the const values

}

private boolean endOfChunk(int position) {
return (position + 1) % CHUNK_SIZE == 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

@@ -266,17 +277,18 @@ public int sizeOfSubProperties() {
@Watched(prefix = "element")
public <V> HugeProperty<?> setProperty(HugeProperty<V> prop) {
if (this.properties == EMPTY_MAP) {
this.properties = new HashMap<>();
this.properties = new IntObjectHashMap<>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we extends IntObjectHashMap and implement Map interface

@@ -266,17 +277,18 @@ public int sizeOfSubProperties() {
@Watched(prefix = "element")
public <V> HugeProperty<?> setProperty(HugeProperty<V> prop) {
if (this.properties == EMPTY_MAP) {
this.properties = new HashMap<>();
this.properties = new IntObjectHashMap<>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also call CollectionFactory.newIntObjectMap()

}

public void resetEdges() {
this.edges = InsertionOrderUtil.newSet();
this.edges = new FastList<>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

call CollectionFactory

if (this.edges == EMPTY_SET) {
this.edges = InsertionOrderUtil.newSet();
if (this.edges == EMPTY_LIST) {
this.edges = new FastList<>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

call CollectionFactory

this.edges = newSet();
}

public void removeEdge(HugeEdge edge) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seem unused, remove this method

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

used in HugeEdge.remove()

private boolean foundPath;

public ShortestPathRecord(Id sourceV, Id targetV) {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove empty line

import com.baidu.hugegraph.util.collection.ObjectIntMapping;
import com.google.common.collect.Lists;

public class PathsRecord implements Record {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it the same function as ArrayPathsRecord

import com.baidu.hugegraph.backend.id.Id;
import com.baidu.hugegraph.traversal.algorithm.HugeTraverser.PathSet;

public interface Record {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer Records

this.targetLayers.peek().keySet().intIterator();
}

public void finishOneLayer() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seem all the subclass are the same mode, abstract it

this.sourceLayers = new Stack<>();
this.targetLayers = new Stack<>();
this.sourceLayers.push(firstSourceLayer);
this.targetLayers.push(firstTargetLayer);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer to split into two layers: fanout and source-target, and abstract source-target layer to the top of all record implement

private PathSet getTargetPath(int target) {
return this.concatPath(this.targetLayers, target,
this.targetLayers.size() - 1);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move these 2 methods to line 152

return results;
}

private PathSet concatPath(Stack<Int2IntsMap> all, int id, int layerIndex) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to linkPath

return results;
}

private PathSet getSourcePath(int source) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to linkSourcePath

private static final int CHUNK_SIZE = 10;
private static final int POSITION_OFFSET = 0;
private static final int SIZE_OFFSET = 1;
private static final int FIRST_CHUNK_DATA_OFFSET = 2;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer prefix with OFFSET_XX

*
*/

private IntIntHashMap firstChunkMap;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

chunkMap is ok

IntIntHashMap layer = this.targetLayers.elementAt(i);
value = layer.get(value);
ids.add(this.id(value));
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also split linkSourcePath() and linkTargetPath()

this.currentLayer.put(target, source);
}

private Path getPath(int source, int target) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also rename to concatPath()

import org.eclipse.collections.api.iterator.IntIterator;
import org.eclipse.collections.impl.map.mutable.primitive.IntIntHashMap;

public class Int2IntsMap {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move to collection package

import com.baidu.hugegraph.util.collection.ObjectIntMapping;
import com.google.common.collect.Lists;

public class ArrayPathsRecords implements Records {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MultiPathsPerVertexRecords or MultiPathsRecords

import com.baidu.hugegraph.util.collection.ObjectIntMapping;
import com.google.common.collect.Lists;

public class PathsRecords implements Records {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MultiPathsBySetRecords


public class IntArrayIterator implements IntIterator {

private int[] array;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set final


public class IntArrayRecord implements Record {

private Int2IntsMap layer;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set final


public class IntIntRecord implements Record {

private IntIntHashMap layer;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set final


public class IntSetRecord implements Record {

private IntObjectHashMap<IntHashSet> layer;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set final


import org.eclipse.collections.api.iterator.IntIterator;

public interface Record {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not addressed: prefer Records

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there already exists Records for multi layers. Record means one layer paths here


package com.baidu.hugegraph.type.define;

public enum CollectionImplType implements SerialEnum {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add unit test

this.sourceRecords = new Stack<>();
this.targetRecords = new Stack<>();
this.sourceRecords.push(firstSourceRecord);
this.targetRecords.push(firstTargetRecord);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can abstract source and target code as 2 instances(of one class)?

public void add(int key, int value) {
if (this.chunkMap.containsKey(key)) {
int firstChunk = this.chunkMap.get(key);
int nextPosition = this.chunkTable[firstChunk + OFFSET_POSITION];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zhoney zhoney force-pushed the collection-optimize branch from 7456243 to 5b56077 Compare May 10, 2021 12:07
return this.customizedKneighbor(source, step, maxDepth,
limit, single);
Traverser traverser = new Traverser(source, step, maxDepth, limit,
single);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to singleThread


List<HugeTraverser.Path> paths = new ArrayList<>();
PathSet paths = new PathSet();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also keep HugeTraverser.PathSet

all.add(sourceV);
private final EdgeStep step;
private final long limit;
private final boolean single;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to concurrent

long limit, boolean single) {
Set<Node> latest = newSet(single);
Set<Node> all = newSet(single);
private class Traverser {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems unneeded to add this class, just add a method?

}

protected long traverseIds(Iterator<Id> ids, Consumer<Id> consumer,
boolean single, boolean stop) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename single to concurrent

default:
throw new AssertionError("Unsupported record type: " + type);
}
if (!single) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto


@Override
public int size() {
return this.record.size();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also synchronized

@@ -31,12 +31,12 @@
private final IntObjectHashMap<V> int2IdMap;

public ObjectIntMapping() {
this.int2IdMap = new IntObjectHashMap<>(1000000);
this.int2IdMap = new IntObjectHashMap<>(10000000);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

define const var

}

@Watched
@SuppressWarnings("unchecked")
public int object2Code(Object object) {
public synchronized int object2Code(Object object) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

always synchronized?

@codecov
Copy link

codecov bot commented May 11, 2021

Codecov Report

Merging #1409 (8b8be47) into master (83f6612) will decrease coverage by 0.51%.
The diff coverage is 32.38%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1409      +/-   ##
============================================
- Coverage     64.05%   63.54%   -0.52%     
- Complexity     5882     5975      +93     
============================================
  Files           381      402      +21     
  Lines         32190    32733     +543     
  Branches       4506     4558      +52     
============================================
+ Hits          20620    20800     +180     
- Misses         9463     9810     +347     
- Partials       2107     2123      +16     
Impacted Files Coverage Δ
.../hugegraph/api/traversers/AllShortestPathsAPI.java 0.00% <ø> (ø)
...m/baidu/hugegraph/api/traversers/KneighborAPI.java 0.00% <0.00%> (ø)
...va/com/baidu/hugegraph/api/traversers/KoutAPI.java 0.00% <0.00%> (ø)
...a/com/baidu/hugegraph/api/traversers/PathsAPI.java 0.00% <ø> (ø)
...aidu/hugegraph/api/traversers/PersonalRankAPI.java 0.00% <ø> (ø)
...aidu/hugegraph/api/traversers/ShortestPathAPI.java 0.00% <ø> (ø)
...m/baidu/hugegraph/api/traversers/TraverserAPI.java 0.00% <ø> (ø)
...a/com/baidu/hugegraph/auth/HugeGraphAuthProxy.java 55.95% <ø> (ø)
...e/src/main/java/com/baidu/hugegraph/HugeGraph.java 65.30% <ø> (ø)
...ugegraph/backend/cache/CachedGraphTransaction.java 84.24% <ø> (ø)
... and 59 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 83f6612...8b8be47. Read the comment docs.

@@ -34,8 +34,8 @@

public KoutRecords(Id source,
RecordType type,
boolean nearest, boolean single) {
super(source, type, nearest, single);
boolean nearest, boolean concurrent) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move "RecordType type" to the first line?

@@ -68,8 +68,8 @@ public SingleWayMultiPathsRecords(Id source, RecordType type,
this.records = new Stack<>();
this.records.push(firstRecord);

this.accessedVertices = single ? new HashSet() :
ConcurrentHashMap.newKeySet();
this.accessedVertices = concurrent ? ConcurrentHashMap.newKeySet() :
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also use primitive map

}

@Watched
@PerfUtil.Watched
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove "PerfUtil."

@@ -55,7 +55,7 @@ public synchronized int object2Code(Object object) {
throw new HugeException("Failed to get code for id: %s", object);
}

@Watched
@PerfUtil.Watched
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

@zhoney zhoney force-pushed the collection-optimize branch from 3a51a42 to be0eaea Compare May 12, 2021 07:27
@zhoney
Copy link
Contributor Author

zhoney commented May 20, 2021

cache tick()

    @Override
    public long tick() {
        long expireTime = this.expire;
        if (expireTime <= 0) {
            return 0L;
        }

        int expireItems = 0;
        long current = now();
        for (Iterator<CacheNode<K, V>> it = this.nodes(); it.hasNext();) {
            CacheNode<K, V> node = it.next();
            LOG.info("expireTime: {}, current: {}, node.time(): {}, " +
                     "current-node.time(): {}, node: {}",
                     expireTime, current, node.time(), current - node.time(), node);
            if (current - node.time() > expireTime) {
                // Remove item while iterating map (it must be ConcurrentMap)
                this.remove(node.key());
                expireItems++;
            }
        }

        if (expireItems > 0) {
            LOG.info("Cache expired {} items cost {}ms (size {}, expire {}ms)",
                      expireItems, now() - current, this.size(), expireTime);
        }
        return expireItems;
    }

output when tick() run:

2021-05-20 04:04:00 241271 [cache-expirer] [INFO ] com.baidu.hugegraph.backend.cache.Cache [] - expireTime: 28000, current: 1621483440642, node.time(): 1621483412909, current-node.time(): 27733, node: fake-id

@zhoney
Copy link
Contributor Author

zhoney commented May 20, 2021

TODO:

  1. primitive集合的多线程加速。目前是使用synchronized的方式加锁粒度比较粗导致性能下降,后续可以采用分段加锁等方式加速
    • ConcurrentObjectIntMapping
    • Record
    • SingleWayMultiPathsRecords.accessedVertices
  2. 改造算法使用primitive的records。目前只有shortestpath、kout、kneighbor和paths改造完成。其他算法仍需要改造。

@zhoney zhoney force-pushed the collection-optimize branch from 1ae39a7 to 4889059 Compare May 20, 2021 08:08
@@ -96,14 +103,19 @@ protected int concurrentDepth() {
return this.graph.option(CoreOptions.OLTP_CONCURRENT_DEPTH);
}

private CollectionImplType collectionImplType() {
return CollectionImplType.valueOf(this.graph.option(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use ConfigConvOption

query.limit(limit);
result = new LimitIterator<>(iterators, e -> {
long count = query.goOffset(1L);
return query.reachLimit(count - 1L);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rebase master

Iterator<Edge> edges = this.edgesOfVertex(source.id(), step);
while (edges.hasNext()) {
Id target = ((HugeEdge) edges.next()).id().otherVertexId();
KNode kNode = new KNode(target, (KNode) source);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can use layer collection?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this method will be deleted.


public class CollectionFactory {

private CollectionImplType type;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to CollectionType

@@ -664,5 +679,9 @@ protected GraphTransaction tx() {
}
return null;
}

private static <V> Set<V> newSet() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move to outer class, also add newList() for HugeVertex.resetEdges()

}

public int size() {
return 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ensure "return 0" is expected?

@zhoney zhoney force-pushed the collection-optimize branch from 4889059 to 224d26b Compare May 21, 2021 04:02
@@ -624,12 +625,13 @@ public static synchronized CoreOptions instance() {
10
);

public static final ConfigOption<String> OLTP_COLLECTION_IMPL_TYPE =
new ConfigOption<>(
public static final ConfigConvOption<String, CollectionType> OLTP_COLLECTION_IMPL_TYPE =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OLTP_COLLECTION_TYPE

@@ -232,7 +236,7 @@ public void removeEdge(HugeEdge edge) {

public void addEdge(HugeEdge edge) {
if (this.edges == EMPTY_LIST) {
this.edges = CollectionFactory.newList(CollectionImplType.EC);
this.edges = CollectionFactory.newList(CollectionType.EC);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

call newList();

private CollectionImplType collectionImplType() {
return CollectionImplType.valueOf(this.graph.option(
CoreOptions.OLTP_COLLECTION_IMPL_TYPE).toUpperCase());
private CollectionType collectionImplType() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

collectionType


List<HugeTraverser.Path> paths = new ArrayList<>();
PathSet paths = new PathSet();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also use HugeTraverser.PathSet

(int) age.id().asLong(),
new HugeVertexProperty<>(vertex, age, 29),
(int) city.id().asLong(),
new HugeVertexProperty<>(vertex, city, "Beijing")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let newIntObjectMap accept <Id,Property>

(int) date.id().asLong(),
new HugeEdgeProperty<>(edge, date, Utils.date("2019-03-12")),
(int) weight.id().asLong(),
new HugeEdgeProperty<>(edge, weight, 0.8)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

@zhoney zhoney force-pushed the collection-optimize branch from 224d26b to b3969e4 Compare May 21, 2021 13:08
E.checkArgument(objects.length % 2 == 0,
"Must provide even arguments for " +
"CollectionFactory.newIntObjectMap");
for (int i = 0; i < objects.length; i+=2) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i += 2

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not addressed: i += 2



/*
* chunkMap chunkTable
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add some spaces before chunkTable to align with the following table


import com.baidu.hugegraph.util.collection.Int2IntsMap;

public class IntArrayRecord implements Record {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Int2ArrayRecord


import org.eclipse.collections.impl.map.mutable.primitive.IntIntHashMap;

public class IntIntRecord implements Record {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Int2IntRecord

import org.eclipse.collections.impl.map.mutable.primitive.IntObjectHashMap;
import org.eclipse.collections.impl.set.mutable.primitive.IntHashSet;

public class IntSetRecord implements Record {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Int2SetRecord

import com.baidu.hugegraph.traversal.algorithm.records.record.RecordType;
import com.google.common.collect.Lists;

public class DoubleWayMultiPathsRecords extends AbstractRecords {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set abstract?

@@ -46,7 +46,7 @@
protected final Stack<Record> records;

private final boolean nearest;
private final MutableIntSet accessedVertices;
protected final MutableIntSet accessedVertices;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep k-out and k-neighbor consistent

@@ -107,6 +108,7 @@
PageStateTest.class,
Int2IntsMapTest.class,
ObjectIntMappingTest.class,
IdSet.class,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IdSetTest

E.checkArgument(objects.length % 2 == 0,
"Must provide even arguments for " +
"CollectionFactory.newIntObjectMap");
for (int i = 0; i < objects.length; i+=2) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not addressed: i += 2

(int) key2.asLong(), value2);
}

public static <V> MutableIntObjectMap<V> newIntObjectMap(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can remove this version method

protected Record currentRecord;
private IntIterator lastRecordKeys;
protected int current;
protected boolean forward;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set fields to private as possible

private IntIterator lastRecordKeys;
protected int current;
protected boolean forward;
private int current;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

means what

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mean int code of currently accessed vertex's Id

@zhoney zhoney force-pushed the collection-optimize branch 2 times, most recently from f400213 to 4392dcb Compare May 28, 2021 07:57

public abstract class HugeElement implements Element, GraphType, Idfiable {

private static final Map<Id, HugeProperty<?>> EMPTY_MAP = ImmutableMap.of();
private static final MutableIntObjectMap<HugeProperty<?>> EMPTY_MAP =
new IntObjectHashMap<>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

= CollectionFactory.newIntObjectMap();

@Watched
@SuppressWarnings("unchecked")
public int object2Code(Object object) {
int key = object.hashCode();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

key -> code

263,264,265,266,267,268,269};
results[4] = new int[]{270,271,272,273,274,275,276,277,278,279,
280,281,282,283,284,285,286,287,288,289,
290, 291,292,293,294,295,296,297,298,299};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

290, 291 -> 290,291


for (int ii = 0; ii < 5; ii++) {
int[] result = map.getValues(ii + 1);
Assert.assertTrue(Arrays.equals(results[ii], result));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assert.assertArrayEquals(results[ii], result);

@zhoney zhoney force-pushed the collection-optimize branch from 4392dcb to 8427bdf Compare June 3, 2021 09:56
Linary
Linary previously approved these changes Jun 4, 2021
zhoney added 21 commits June 8, 2021 14:18
Change-Id: I6c13f504986183804f4c7ae45a5f4e0e6099125b
Change-Id: Ie8b79a4c05696f1842476bd74d82b75fba4925a1
Change-Id: I7509a653ad429d329165e968358cf4d01bf86742
Change-Id: I4b79d0987f1eebcfb952bc4627a2f4fc8d2dcaf3
Change-Id: Iffad898d0ff04f8b8937eb774352c853bae87aef
Change-Id: I0791e3d307f2cff51ccc4760ea63c52468a4b8d8
Change-Id: I9e5926333dc6ad12fd081695e37994d77f26faab
Change-Id: I761abfa4940ebaa1dd331e9c60c80293bea5d2cb
Change-Id: Ib653623d5cc79696f58ec50ccadca8ac7039c2f1
Change-Id: I742ce92cea71f556d0c7b5a52229e46006f759e7
Change-Id: Ia6a8e3316c40ecf44f6f36c4cfee50db2f7d11fe
Change-Id: I9588f2df9a6db197dcd0d35c98b7d99993a9aabf
Change-Id: I1358fb9216dd2a8d04be0e8772cd715c79c61b1e
Change-Id: Icb18c665b27dced23415bdcfc582fa372d1fe2ba
Change-Id: I5ca0ed20f0bba31226aeb10e22d191bc970e6805
Change-Id: I40b3e2c5b3d66206045ce6842e2a85ad2907bcdd
Change-Id: Ieb9a8bb4a0c2e7c6f2f18f8745783c93e51754cf
Change-Id: I1a66e5ea99a32f6cead79b80317dc7d7b3a77fa7
Change-Id: I80bd3d4ae961794cc82139262d605fe400f8c28b
Change-Id: I14abe49850d42f34576c6d1837ed5b620ffbda21
Change-Id: I580df88f57302dfe474108a93108759e33e083e2
@zhoney zhoney merged commit 23e3c31 into master Jun 9, 2021
@zhoney zhoney deleted the collection-optimize branch June 9, 2021 09:36
jadepeng pushed a commit to jadepeng/hugegraph that referenced this pull request Jun 25, 2021
* add Eclipse Collection dependency
* use primitive map for properties of HugeElement
* from Map<Id, HugeProperty<?>> to MutableIntObjectMap<HugeProperty<?>>
* allow config oltp collections(JCF or EC)
* add fastutil choice for collection
* change Set<HugeEdge> of HugeVertex to List<HugeEdge>
* use IntObjectMap and IntIntMap to improve shortest path
* improve paths api by remove redundant sourcesAll and targetsAll
* improve paths api performance by using IntObjectHashMap<IntHashSet>
* extract shortestpath and paths record to hold traversed paths
* implement Int2IntsMap for array paths record
* extract Record and Records for paths and shortest path
* adapt primitive collection for kout/kneighbor
* add objectintmapping unit test
* add UT for CollectionFactory and IdSet

Change-Id: I501a18a70054fad3c40bd6b9e852a82a5c185949
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants