javeme commented on code in PR #2262:
URL:
https://github.com/apache/incubator-hugegraph/pull/2262#discussion_r1278570485
##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java:
##########
@@ -190,4 +192,42 @@ public static boolean checkAndParseAction(String action) {
String.format("Not support action '%s'", action));
}
}
+
+ public static class ApiMeasure {
Review Comment:
prefer rename to ApiMeasurer and expect an blank line after a class define
##########
hugegraph-core/src/main/java/org/apache/hugegraph/traversal/algorithm/TemplatePathsTraverser.java:
##########
@@ -276,4 +280,26 @@ public boolean lastSuperStep() {
this.targetIndex == this.sourceIndex + 1;
}
}
+
+ public static class WrappedPathSet {
Review Comment:
ditto
##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java:
##########
@@ -190,4 +192,42 @@ public static boolean checkAndParseAction(String action) {
String.format("Not support action '%s'", action));
}
}
+
+ public static class ApiMeasure {
+ public static final String EDGE_ITER = "edge_iters";
+ public static final String VERTICE_ITER = "vertice_iters";
+ public static final String COST = "cost";
+ protected long timeStart = System.currentTimeMillis();
+ protected HashMap<String, Object> mapResult = new LinkedHashMap<>();
+
+ public Map<String, Object> getResult() {
+ mapResult.put(COST, System.currentTimeMillis() - timeStart);
+ return mapResult;
+ }
+
+ public void put(String key, String value) {
+ this.mapResult.put(key, value);
+ }
+
+ public void put(String key, long value) {
+ this.mapResult.put(key, value);
+ }
+
+ public void put(String key, int value) {
+ this.mapResult.put(key, value);
+ }
+
+ protected void addCount(String key, long value) {
+ long cur = 0;
Review Comment:
prefer current
##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/KneighborAPI.java:
##########
@@ -94,8 +100,14 @@ public String get(@Context GraphManager manager,
try (KneighborTraverser traverser = new KneighborTraverser(g)) {
ids = traverser.kneighbor(source, dir, edgeLabel,
depth, maxDegree, limit);
+ measure.addIterCount(traverser.vertexIterCounter.get(),
+ traverser.edgeIterCounter.get());
+ }
+ if (countOnly) {
+ return manager.serializer(g, measure.getResult()).writeMap(
Review Comment:
prefer to wrap line before ".writeMap"
##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java:
##########
@@ -190,4 +192,42 @@ public static boolean checkAndParseAction(String action) {
String.format("Not support action '%s'", action));
}
}
+
+ public static class ApiMeasure {
+ public static final String EDGE_ITER = "edge_iters";
+ public static final String VERTICE_ITER = "vertice_iters";
+ public static final String COST = "cost";
+ protected long timeStart = System.currentTimeMillis();
+ protected HashMap<String, Object> mapResult = new LinkedHashMap<>();
Review Comment:
mark private as possible and assign in the constructor
##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java:
##########
@@ -190,4 +192,42 @@ public static boolean checkAndParseAction(String action) {
String.format("Not support action '%s'", action));
}
}
+
+ public static class ApiMeasure {
+ public static final String EDGE_ITER = "edge_iters";
+ public static final String VERTICE_ITER = "vertice_iters";
+ public static final String COST = "cost";
+ protected long timeStart = System.currentTimeMillis();
+ protected HashMap<String, Object> mapResult = new LinkedHashMap<>();
+
+ public Map<String, Object> getResult() {
+ mapResult.put(COST, System.currentTimeMillis() - timeStart);
+ return mapResult;
+ }
+
+ public void put(String key, String value) {
+ this.mapResult.put(key, value);
+ }
+
+ public void put(String key, long value) {
+ this.mapResult.put(key, value);
+ }
+
+ public void put(String key, int value) {
+ this.mapResult.put(key, value);
+ }
+
+ protected void addCount(String key, long value) {
+ long cur = 0;
+ if (this.mapResult.containsKey(key)) {
+ cur = (long) this.mapResult.get(key);
+ }
+ this.mapResult.put(key, cur + value);
Review Comment:
can we use a MutableLong instead? then we don't need to put map every time
##########
hugegraph-core/src/main/java/org/apache/hugegraph/traversal/algorithm/SingleSourceShortestPathTraverser.java:
##########
@@ -91,34 +102,129 @@ public NodeWithWeight weightedShortestPath(Id sourceV, Id
targetV,
traverser.forward();
Map<Id, NodeWithWeight> results = traverser.shortestPaths();
if (results.containsKey(targetV) || traverser.done()) {
- return results.get(targetV);
+ this.vertexIterCounter.addAndGet(traverser.vertexCount);
+ this.edgeIterCounter.addAndGet(traverser.edgeCount);
+ NodeWithWeight nodeWithWeight = results.get(targetV);
+ if (nodeWithWeight != null) {
+ Iterator<Id> vertexIter =
nodeWithWeight.node.path().iterator();
+ Set<Edge> edges =
traverser.edgeRecord.getEdges(vertexIter);
+ nodeWithWeight.setEdges(edges);
+ }
+ return nodeWithWeight;
}
checkCapacity(traverser.capacity, traverser.size, "shortest path");
}
}
+ public static class NodeWithWeight implements Comparable<NodeWithWeight> {
+
+ private final double weight;
+ private final Node node;
+
+ private Set<Edge> edges = Collections.emptySet();
+
+ public NodeWithWeight(double weight, Node node) {
+ this.weight = weight;
+ this.node = node;
+ }
+
+ public NodeWithWeight(double weight, Id id, NodeWithWeight prio) {
+ this(weight, new Node(id, prio.node()));
+ }
+
+ public Set<Edge> getEdges() {
+ return edges;
+ }
+
+ public void setEdges(Set<Edge> edges) {
+ this.edges = edges;
+ }
+
+ public double weight() {
+ return weight;
+ }
+
+ public Node node() {
+ return this.node;
+ }
+
+ public Map<String, Object> toMap() {
+ return ImmutableMap.of("weight", this.weight,
+ "vertices", this.node().path());
+ }
+
+ @Override
+ public int compareTo(NodeWithWeight other) {
+ return Double.compare(this.weight, other.weight);
+ }
+ }
+
+ public static class WeightedPaths extends LinkedHashMap<Id,
NodeWithWeight> {
+
+ private static final long serialVersionUID = -313873642177730993L;
+ private Set<Edge> edges = Collections.emptySet();
+
+ public Set<Edge> getEdges() {
+ return edges;
+ }
+
+ public void setEdges(Set<Edge> edges) {
+ this.edges = edges;
+ }
Review Comment:
can keep "edges()" name style, please also update other places
##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java:
##########
@@ -190,4 +192,42 @@ public static boolean checkAndParseAction(String action) {
String.format("Not support action '%s'", action));
}
}
+
+ public static class ApiMeasure {
+ public static final String EDGE_ITER = "edge_iters";
Review Comment:
rename to edge_iterations?
##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java:
##########
@@ -190,4 +192,42 @@ public static boolean checkAndParseAction(String action) {
String.format("Not support action '%s'", action));
}
}
+
+ public static class ApiMeasure {
+ public static final String EDGE_ITER = "edge_iters";
+ public static final String VERTICE_ITER = "vertice_iters";
+ public static final String COST = "cost";
+ protected long timeStart = System.currentTimeMillis();
+ protected HashMap<String, Object> mapResult = new LinkedHashMap<>();
+
+ public Map<String, Object> getResult() {
Review Comment:
prefer `measures()` name
##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/API.java:
##########
@@ -190,4 +192,42 @@ public static boolean checkAndParseAction(String action) {
String.format("Not support action '%s'", action));
}
}
+
+ public static class ApiMeasure {
+ public static final String EDGE_ITER = "edge_iters";
+ public static final String VERTICE_ITER = "vertice_iters";
+ public static final String COST = "cost";
+ protected long timeStart = System.currentTimeMillis();
+ protected HashMap<String, Object> mapResult = new LinkedHashMap<>();
Review Comment:
rename to measures? and we can use InsertionOrderUtil.newMap()
##########
hugegraph-core/src/main/java/org/apache/hugegraph/traversal/algorithm/CollectionPathsTraverser.java:
##########
@@ -274,4 +280,26 @@ protected int accessedNodes() {
return this.sourcesAll.size() + this.targetsAll.size();
}
}
+
+ public static class WrappedPathCollection {
Review Comment:
ditto
##########
hugegraph-core/src/main/java/org/apache/hugegraph/traversal/algorithm/MultiNodeShortestPathTraverser.java:
##########
@@ -56,70 +83,69 @@ public List<Path> multiNodeShortestPath(Iterator<Vertex>
vertices,
});
if (maxDepth >= this.concurrentDepth() && vertexCount > 10) {
- return this.multiNodeShortestPathConcurrent(pairs, step,
- maxDepth, capacity);
+ return this.multiNodeShortestPathConcurrent(pairs, step, maxDepth,
capacity);
} else {
- return this.multiNodeShortestPathSingle(pairs, step,
- maxDepth, capacity);
+ return this.multiNodeShortestPathSingle(pairs, step, maxDepth,
capacity);
}
}
- public List<Path> multiNodeShortestPathConcurrent(List<Pair<Id, Id>> pairs,
- EdgeStep step,
- int maxDepth,
- long capacity) {
- List<Path> results = new CopyOnWriteArrayList<>();
+ public WrappedListPath multiNodeShortestPathConcurrent(List<Pair<Id, Id>>
pairs,
+ EdgeStep step, int
maxDepth,
+ long capacity) {
+ List<Path> paths = new CopyOnWriteArrayList<>();
+ Set<Edge> edges = new CopyOnWriteArraySet<>();
ShortestPathTraverser traverser =
- new ShortestPathTraverser(this.graph());
+ new ShortestPathTraverser(this.graph());
this.traversePairs(pairs.iterator(), pair -> {
Path path = traverser.shortestPath(pair.getLeft(), pair.getRight(),
step, maxDepth, capacity);
if (!Path.EMPTY.equals(path)) {
- results.add(path);
+ paths.add(path);
}
+ edges.addAll(path.getEdges());
});
+ this.vertexIterCounter.addAndGet(traverser.vertexIterCounter.get());
+ this.edgeIterCounter.addAndGet(traverser.edgeIterCounter.get());
- return results;
+ return new WrappedListPath(paths, edges);
}
- public List<Path> multiNodeShortestPathSingle(List<Pair<Id, Id>> pairs,
- EdgeStep step, int maxDepth,
- long capacity) {
- List<Path> results = newList();
+ public WrappedListPath multiNodeShortestPathSingle(List<Pair<Id, Id>>
pairs,
+ EdgeStep step, int
maxDepth,
+ long capacity) {
+ List<Path> paths = newList();
+ Set<Edge> edges = newSet();
ShortestPathTraverser traverser =
- new ShortestPathTraverser(this.graph());
+ new ShortestPathTraverser(this.graph());
for (Pair<Id, Id> pair : pairs) {
Path path = traverser.shortestPath(pair.getLeft(), pair.getRight(),
step, maxDepth, capacity);
if (!Path.EMPTY.equals(path)) {
- results.add(path);
+ paths.add(path);
}
+ edges.addAll(path.getEdges());
}
- return results;
+ this.vertexIterCounter.addAndGet(traverser.vertexIterCounter.get());
+ this.edgeIterCounter.addAndGet(traverser.edgeIterCounter.get());
+
+ return new WrappedListPath(paths, edges);
}
- private static <T> void cmn(List<T> all, int m, int n, int current,
- List<T> result, Consumer<List<T>> consumer) {
- assert m <= all.size();
- assert current <= all.size();
- if (result == null) {
- result = newList(n);
- }
- if (n == 0) {
- // All n items are selected
- consumer.accept(result);
- return;
+ public static class WrappedListPath {
Review Comment:
ditto
##########
hugegraph-core/src/main/java/org/apache/hugegraph/traversal/algorithm/TemplatePathsTraverser.java:
##########
@@ -276,4 +280,26 @@ public boolean lastSuperStep() {
this.targetIndex == this.sourceIndex + 1;
}
}
+
+ public static class WrappedPathSet {
+ private final Set<Path> paths;
+ private Set<Edge> edges = Collections.emptySet();
+
+ public WrappedPathSet(Set<Path> paths) {
+ this.paths = paths;
+ }
+
+ public WrappedPathSet(Set<Path> paths, Set<Edge> edges) {
+ this.paths = paths;
+ this.edges = edges;
+ }
+
+ public Set<Path> getPaths() {
+ return paths;
+ }
+
+ public Set<Edge> getEdges() {
+ return edges;
+ }
Review Comment:
ditto
##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/AllShortestPathsAPI.java:
##########
@@ -86,8 +97,34 @@ public String get(@Context GraphManager manager,
List<String> edgeLabels = edgeLabel == null ? ImmutableList.of() :
ImmutableList.of(edgeLabel);
HugeTraverser.PathSet paths = traverser.allShortestPaths(
- sourceId, targetId, dir, edgeLabels,
- depth, maxDegree, skipDegree, capacity);
- return manager.serializer(g).writePaths("paths", paths, false);
+ sourceId, targetId, dir, edgeLabels,
Review Comment:
prefer keep the old style
##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/CustomizedCrosspointsAPI.java:
##########
@@ -22,45 +22,62 @@
import static
org.apache.hugegraph.traversal.algorithm.HugeTraverser.DEFAULT_PATHS_LIMIT;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
-import io.swagger.v3.oas.annotations.tags.Tag;
-import jakarta.inject.Singleton;
-import jakarta.ws.rs.Consumes;
-import jakarta.ws.rs.POST;
-import jakarta.ws.rs.Path;
-import jakarta.ws.rs.PathParam;
-import jakarta.ws.rs.Produces;
-import jakarta.ws.rs.core.Context;
-
-import org.apache.tinkerpop.gremlin.structure.Vertex;
-import org.apache.hugegraph.core.GraphManager;
-import org.slf4j.Logger;
-
import org.apache.hugegraph.HugeGraph;
import org.apache.hugegraph.api.API;
import org.apache.hugegraph.backend.id.Id;
-import org.apache.hugegraph.backend.query.QueryResults;
+import org.apache.hugegraph.core.GraphManager;
import org.apache.hugegraph.traversal.algorithm.CustomizedCrosspointsTraverser;
import org.apache.hugegraph.traversal.algorithm.HugeTraverser;
import org.apache.hugegraph.type.define.Directions;
import org.apache.hugegraph.util.E;
import org.apache.hugegraph.util.Log;
+import org.apache.tinkerpop.gremlin.structure.Edge;
+import org.apache.tinkerpop.gremlin.structure.Vertex;
+import org.slf4j.Logger;
+
import com.codahale.metrics.annotation.Timed;
import com.fasterxml.jackson.annotation.JsonAlias;
import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.tags.Tag;
+import jakarta.inject.Singleton;
+import jakarta.ws.rs.Consumes;
+import jakarta.ws.rs.POST;
+import jakarta.ws.rs.Path;
+import jakarta.ws.rs.PathParam;
+import jakarta.ws.rs.Produces;
+import jakarta.ws.rs.core.Context;
+
@Path("graphs/{graph}/traversers/customizedcrosspoints")
@Singleton
@Tag(name = "CustomizedCrosspointsAPI")
public class CustomizedCrosspointsAPI extends API {
private static final Logger LOG =
Log.logger(CustomizedCrosspointsAPI.class);
+ private static List<CustomizedCrosspointsTraverser.PathPattern>
pathPatterns(
+ HugeGraph graph, CrosspointsRequest request) {
+ int stepSize = request.pathPatterns.size();
+ List<CustomizedCrosspointsTraverser.PathPattern> pathPatterns;
+ pathPatterns = new ArrayList<>(stepSize);
+ for (PathPattern pattern : request.pathPatterns) {
+ CustomizedCrosspointsTraverser.PathPattern pathPattern;
+ pathPattern = new CustomizedCrosspointsTraverser.PathPattern();
Review Comment:
ditto
##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/CustomizedCrosspointsAPI.java:
##########
@@ -22,45 +22,62 @@
import static
org.apache.hugegraph.traversal.algorithm.HugeTraverser.DEFAULT_PATHS_LIMIT;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
-import io.swagger.v3.oas.annotations.tags.Tag;
-import jakarta.inject.Singleton;
-import jakarta.ws.rs.Consumes;
-import jakarta.ws.rs.POST;
-import jakarta.ws.rs.Path;
-import jakarta.ws.rs.PathParam;
-import jakarta.ws.rs.Produces;
-import jakarta.ws.rs.core.Context;
-
-import org.apache.tinkerpop.gremlin.structure.Vertex;
-import org.apache.hugegraph.core.GraphManager;
-import org.slf4j.Logger;
-
import org.apache.hugegraph.HugeGraph;
import org.apache.hugegraph.api.API;
import org.apache.hugegraph.backend.id.Id;
-import org.apache.hugegraph.backend.query.QueryResults;
+import org.apache.hugegraph.core.GraphManager;
import org.apache.hugegraph.traversal.algorithm.CustomizedCrosspointsTraverser;
import org.apache.hugegraph.traversal.algorithm.HugeTraverser;
import org.apache.hugegraph.type.define.Directions;
import org.apache.hugegraph.util.E;
import org.apache.hugegraph.util.Log;
+import org.apache.tinkerpop.gremlin.structure.Edge;
+import org.apache.tinkerpop.gremlin.structure.Vertex;
+import org.slf4j.Logger;
+
import com.codahale.metrics.annotation.Timed;
import com.fasterxml.jackson.annotation.JsonAlias;
import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.tags.Tag;
+import jakarta.inject.Singleton;
+import jakarta.ws.rs.Consumes;
+import jakarta.ws.rs.POST;
+import jakarta.ws.rs.Path;
+import jakarta.ws.rs.PathParam;
+import jakarta.ws.rs.Produces;
+import jakarta.ws.rs.core.Context;
+
@Path("graphs/{graph}/traversers/customizedcrosspoints")
@Singleton
@Tag(name = "CustomizedCrosspointsAPI")
public class CustomizedCrosspointsAPI extends API {
private static final Logger LOG =
Log.logger(CustomizedCrosspointsAPI.class);
+ private static List<CustomizedCrosspointsTraverser.PathPattern>
pathPatterns(
+ HugeGraph graph, CrosspointsRequest request) {
+ int stepSize = request.pathPatterns.size();
+ List<CustomizedCrosspointsTraverser.PathPattern> pathPatterns;
+ pathPatterns = new ArrayList<>(stepSize);
Review Comment:
one line is ok
##########
hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/CrosspointsAPI.java:
##########
@@ -84,6 +85,9 @@ public String get(@Context GraphManager manager,
dir, edgeLabel, depth,
maxDegree, capacity,
limit);
- return manager.serializer(g).writePaths("crosspoints", paths, true);
+ measure.addIterCount(traverser.vertexIterCounter.get(),
+ traverser.edgeIterCounter.get());
+ return manager.serializer(g, measure.getResult())
Review Comment:
prefer to update the `serializer(g)` method and set default an ApiMeasure
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
