----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10052/#review18427 -----------------------------------------------------------
Looking good. See inline comments. Further suggestions for eliminating the intermediate map storage: - the internal storage for TestGraph will be a Map<I, Vertex> - addVertex(I, V) will add a Vertex with the given id and value and no edges, using the GiraphClasses instance - addVertex(I, V, Iterable<Edge<I, E>>) will add a Vertex with the given edges - addEdge(I, I, E) will add an edge from the first vertex to the second vertex, with the given edge value; if the source vertex doesn't exist, it will be created - TestGraph will implement Iterable<Vertex> instead of Iterable<I> - an additional getVertex(I) method will be used for random access - the InMemoryVertexInputFormat will simply pass references to the already-created vertices - no output format is needed; run() will return the same TestGraph instance giraph-core/src/main/java/org/apache/giraph/utils/InMemoryVertexInput.java <https://reviews.apache.org/r/10052/#comment38637> I would probably rename this class to InMemoryVertexInputFormat and change the comment to "An input format that reads the input graph in memory. Used for unit tests." or something like that. giraph-core/src/main/java/org/apache/giraph/utils/InMemoryVertexInput.java <https://reviews.apache.org/r/10052/#comment38638> Can you move this method down, close to setConf()? giraph-core/src/main/java/org/apache/giraph/utils/InMemoryVertexInput.java <https://reviews.apache.org/r/10052/#comment38635> Missing a newline. giraph-core/src/main/java/org/apache/giraph/utils/InMemoryVertexInput.java <https://reviews.apache.org/r/10052/#comment38639> Style: we prefer more explicit/verbose naming here. i -> id v -> value es -> edges giraph-core/src/main/java/org/apache/giraph/utils/InMemoryVertexInput.java <https://reviews.apache.org/r/10052/#comment38636> Extra newline. giraph-core/src/main/java/org/apache/giraph/utils/InMemoryVertexInput.java <https://reviews.apache.org/r/10052/#comment38640> is -> inputSplit c - > context giraph-core/src/main/java/org/apache/giraph/utils/InMemoryVertexInput.java <https://reviews.apache.org/r/10052/#comment38641> I would rename this to something like idIterator. giraph-core/src/main/java/org/apache/giraph/utils/InMemoryVertexInput.java <https://reviews.apache.org/r/10052/#comment38642> c -> context giraph-core/src/main/java/org/apache/giraph/utils/InMemoryVertexOutput.java <https://reviews.apache.org/r/10052/#comment38643> InMemoryVertexOutputFormat? giraph-core/src/main/java/org/apache/giraph/utils/InMemoryVertexOutput.java <https://reviews.apache.org/r/10052/#comment38645> Extra newline. giraph-core/src/main/java/org/apache/giraph/utils/InMemoryVertexOutput.java <https://reviews.apache.org/r/10052/#comment38644> Can't you use the ImmutableOutputCommitter here? giraph-core/src/main/java/org/apache/giraph/utils/TestGraph.java <https://reviews.apache.org/r/10052/#comment38646> Maps.newHashMap() is more compact. This applies to pretty much all collections here (Lists.newArrayList() and so on). giraph-core/src/main/java/org/apache/giraph/utils/TestGraph.java <https://reviews.apache.org/r/10052/#comment38647> neighbours -> edges giraph-core/src/main/java/org/apache/giraph/utils/TestGraph.java <https://reviews.apache.org/r/10052/#comment38648> "Iterator over the edges for a given vertex id" - Alessandro Presta On March 26, 2013, 11:30 p.m., Veselin Stoyanov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/10052/ > ----------------------------------------------------------- > > (Updated March 26, 2013, 11:30 p.m.) > > > Review request for giraph. > > > Description > ------- > > A framework for running small test graphs in memory. > > > Diffs > ----- > > giraph-core/src/main/java/org/apache/giraph/utils/InMemoryVertexInput.java > PRE-CREATION > giraph-core/src/main/java/org/apache/giraph/utils/InMemoryVertexOutput.java > PRE-CREATION > giraph-core/src/main/java/org/apache/giraph/utils/InternalVertexRunner.java > 7e0b955998a5f1890912819fa2ca74fbbf46fedd > giraph-core/src/main/java/org/apache/giraph/utils/TestGraph.java > PRE-CREATION > > giraph-examples/src/test/java/org/apache/giraph/examples/ConnectedComponentsVertexTestInMemory.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/10052/diff/ > > > Testing > ------- > > > Thanks, > > Veselin Stoyanov > >
