giraph-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alessandro Presta" <alessan...@fb.com>
Subject Re: Review Request: In memory testing framework added
Date Wed, 27 Mar 2013 18:00:01 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message