You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@giraph.apache.org by "Avery Ching (Created) (JIRA)" <ji...@apache.org> on 2012/01/01 00:21:30 UTC

[jira] [Created] (GIRAPH-116) Make EdgeListVertex the default vertex implementation

Make EdgeListVertex the default vertex implementation
-----------------------------------------------------

                 Key: GIRAPH-116
                 URL: https://issues.apache.org/jira/browse/GIRAPH-116
             Project: Giraph
          Issue Type: Improvement
            Reporter: Avery Ching
            Assignee: Avery Ching


I think this would best for new users as it is much more memory efficient than Vertex with respect to edges (list vs hash map).  We seem to be mostly iterating over the edges (as several others had pointed out in earlier JIRAs and emails), so this would provide early users with a more memory efficient implementation without performance loss.  If anyone disagrees, please voice your opinions.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (GIRAPH-116) Make EdgeListVertex the default vertex implementation, fix bugs related to EdgeListVertex.

Posted by "Hudson (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/GIRAPH-116?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13178514#comment-13178514 ] 

Hudson commented on GIRAPH-116:
-------------------------------

Integrated in Giraph-trunk-Commit #60 (See [https://builds.apache.org/job/Giraph-trunk-Commit/60/])
    GIRAPH-116. Forgot to remove this.
GIRAPH-116: Make EdgeListVertex the default vertex implementation, fix
bugs related to EdgeListVertex. (aching)

aching : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1226518
Files : 
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/Vertex.java

aching : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1226513
Files : 
* /incubator/giraph/trunk/CHANGELOG
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PageRankBenchmark.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PseudoRandomVertexInputFormat.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/CommunicationsInterface.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerCommunications.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCombinerVertex.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleFailVertex.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMsgVertex.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMutateGraphVertex.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleShortestPathsVertex.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleSuperstepVertex.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleVertexWithWorkerContext.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/VerifyMessage.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertexResolver.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspUtils.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/EdgeListVertex.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/HashMapVertex.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/IntIntNullIntVertex.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/MutableVertex.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/VertexResolver.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/Partition.java
* /incubator/giraph/trunk/src/test/java/org/apache/giraph/TestBspBasic.java
* /incubator/giraph/trunk/src/test/java/org/apache/giraph/TestJsonBase64Format.java
* /incubator/giraph/trunk/src/test/java/org/apache/giraph/TestVertexTypes.java
* /incubator/giraph/trunk/src/test/java/org/apache/giraph/examples/SimpleShortestPathVertexTest.java
* /incubator/giraph/trunk/src/test/java/org/apache/giraph/graph/TestEdgeListVertex.java
* /incubator/giraph/trunk/src/test/java/org/apache/giraph/lib/TestLongDoubleDoubleAdjacencyListVertexInputFormat.java
* /incubator/giraph/trunk/src/test/java/org/apache/giraph/lib/TestTextDoubleDoubleAdjacencyListVertexInputFormat.java
* /incubator/giraph/trunk/src/test/java/org/apache/giraph/utils/MockUtils.java

                
> Make EdgeListVertex the default vertex implementation, fix bugs related to EdgeListVertex.
> ------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-116
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-116
>             Project: Giraph
>          Issue Type: Improvement
>            Reporter: Avery Ching
>            Assignee: Avery Ching
>         Attachments: GIRAPH-116.patch
>
>
> I think this would best for new users as it is much more memory efficient than Vertex with respect to edges (list vs hash map).  We seem to be mostly iterating over the edges (as several others had pointed out in earlier JIRAs and emails), so this would provide early users with a more memory efficient implementation without performance loss.  If anyone disagrees, please voice your opinions.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (GIRAPH-116) Make EdgeListVertex the default vertex implementation, fix bugs related to EdgeListVertex.

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/GIRAPH-116?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13178348#comment-13178348 ] 

jiraposter@reviews.apache.org commented on GIRAPH-116:
------------------------------------------------------


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3349/#review4168
-----------------------------------------------------------


Had a quick look over your changes and everything looked good. I think it's right to assume that most implementations will use static graphs and to offer EdgeListVertex as the default extension point for this. The only thing I don't like is the name change from MutableVertex to BasicVertex, I liked the former better because it sounds much more expressive to me.


http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java
<https://reviews.apache.org/r/3349/#comment9400>

    good thing to have the Iterable<> abstraction here


- Sebastian


On 2012-01-02 02:35:50, Avery Ching wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/3349/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2012-01-02 02:35:50)
bq.  
bq.  
bq.  Review request for giraph.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  * Changed Vertex.java to HashMapVertex.java.  This makes it less likely folks will use it as a default.  I have included comments that suggest EdgeListVertex for static graphs (most cases).
bq.  
bq.  * Found and fixed bugs in EdgeListVertex with the way that binarySearch was being used.  Added unittests to check for adding/getting/removing edges.
bq.  
bq.  * Changed classes that extend Vertex to extending EdgeListVertex instead.
bq.  
bq.  * Changed MutableVertex to BasicVertex for addVertex, addVertexReq to be a little safer
bq.  
bq.  * Tried to make sure that when a class that extends MutableVertex is instantiated that it also will call readFields() or initialize().  This fixed several bugs.
bq.  
bq.  * Changed the interface of BasicVertex#initialize from  public abstract void initialize(I vertexId, V vertexValue, Map<I, E> edges, List<M> messages) to 
bq.  initialize(I vertexId, V vertexValue, Map<I, E> edges, Iterable<M> messages) to better fit the recent changes to BasicVertex getting/setting messages with an Iterable.
bq.  
bq.  * Found and removed duplicated code from several MutableVertex extended classes for addVertexRequest, removeVertexRequest, addEdgeRequest and removeEdgeRequest.
bq.  
bq.  * Changed Vertex cast to BasicVertex cast in Partition and MockUtils.
bq.  
bq.  * There are some tabs --> spaces conversions done automatically from my Ecipse settings for the files I touched.
bq.  
bq.  
bq.  This addresses bug GIRAPH-116.
bq.      https://issues.apache.org/jira/browse/GIRAPH-116
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PageRankBenchmark.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PseudoRandomVertexInputFormat.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/CommunicationsInterface.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerCommunications.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCombinerVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleFailVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMsgVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMutateGraphVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleShortestPathsVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleSuperstepVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleVertexWithWorkerContext.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/VerifyMessage.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertexResolver.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspUtils.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/EdgeListVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/HashMapVertex.java PRE-CREATION 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/IntIntNullIntVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/MutableVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/VertexResolver.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/Partition.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestBspBasic.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestJsonBase64Format.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestVertexTypes.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/examples/SimpleShortestPathVertexTest.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/lib/TestLongDoubleDoubleAdjacencyListVertexInputFormat.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/lib/TestTextDoubleDoubleAdjacencyListVertexInputFormat.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/utils/MockUtils.java 1226330 
bq.  
bq.  Diff: https://reviews.apache.org/r/3349/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  Passed unittests and pseudo-distributed MR tests.  Passed newly added unittests as well for EdgeListVertex.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Avery
bq.  
bq.


                
> Make EdgeListVertex the default vertex implementation, fix bugs related to EdgeListVertex.
> ------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-116
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-116
>             Project: Giraph
>          Issue Type: Improvement
>            Reporter: Avery Ching
>            Assignee: Avery Ching
>         Attachments: GIRAPH-116.patch
>
>
> I think this would best for new users as it is much more memory efficient than Vertex with respect to edges (list vs hash map).  We seem to be mostly iterating over the edges (as several others had pointed out in earlier JIRAs and emails), so this would provide early users with a more memory efficient implementation without performance loss.  If anyone disagrees, please voice your opinions.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (GIRAPH-116) Make EdgeListVertex the default vertex implementation, fix bugs related to EdgeListVertex.

Posted by "Avery Ching (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/GIRAPH-116?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Avery Ching updated GIRAPH-116:
-------------------------------

    Attachment: GIRAPH-116.patch

Turns out this was a bit more involved as there were some bugs with EdgeListVertex.  Also, cleaned up the vertex hierarchy as well as part of these changes.
                
> Make EdgeListVertex the default vertex implementation, fix bugs related to EdgeListVertex.
> ------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-116
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-116
>             Project: Giraph
>          Issue Type: Improvement
>            Reporter: Avery Ching
>            Assignee: Avery Ching
>         Attachments: GIRAPH-116.patch
>
>
> I think this would best for new users as it is much more memory efficient than Vertex with respect to edges (list vs hash map).  We seem to be mostly iterating over the edges (as several others had pointed out in earlier JIRAs and emails), so this would provide early users with a more memory efficient implementation without performance loss.  If anyone disagrees, please voice your opinions.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (GIRAPH-116) Make EdgeListVertex the default vertex implementation, fix bugs related to EdgeListVertex.

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/GIRAPH-116?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13178501#comment-13178501 ] 

jiraposter@reviews.apache.org commented on GIRAPH-116:
------------------------------------------------------


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3349/#review4170
-----------------------------------------------------------

Ship it!


Ok, I understood this wrong. I'm fine with the changes then.

- Sebastian


On 2012-01-02 02:35:50, Avery Ching wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/3349/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2012-01-02 02:35:50)
bq.  
bq.  
bq.  Review request for giraph.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  * Changed Vertex.java to HashMapVertex.java.  This makes it less likely folks will use it as a default.  I have included comments that suggest EdgeListVertex for static graphs (most cases).
bq.  
bq.  * Found and fixed bugs in EdgeListVertex with the way that binarySearch was being used.  Added unittests to check for adding/getting/removing edges.
bq.  
bq.  * Changed classes that extend Vertex to extending EdgeListVertex instead.
bq.  
bq.  * Changed MutableVertex to BasicVertex for addVertex, addVertexReq to be a little safer
bq.  
bq.  * Tried to make sure that when a class that extends MutableVertex is instantiated that it also will call readFields() or initialize().  This fixed several bugs.
bq.  
bq.  * Changed the interface of BasicVertex#initialize from  public abstract void initialize(I vertexId, V vertexValue, Map<I, E> edges, List<M> messages) to 
bq.  initialize(I vertexId, V vertexValue, Map<I, E> edges, Iterable<M> messages) to better fit the recent changes to BasicVertex getting/setting messages with an Iterable.
bq.  
bq.  * Found and removed duplicated code from several MutableVertex extended classes for addVertexRequest, removeVertexRequest, addEdgeRequest and removeEdgeRequest.
bq.  
bq.  * Changed Vertex cast to BasicVertex cast in Partition and MockUtils.
bq.  
bq.  * There are some tabs --> spaces conversions done automatically from my Ecipse settings for the files I touched.
bq.  
bq.  
bq.  This addresses bug GIRAPH-116.
bq.      https://issues.apache.org/jira/browse/GIRAPH-116
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PageRankBenchmark.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PseudoRandomVertexInputFormat.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/CommunicationsInterface.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerCommunications.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCombinerVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleFailVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMsgVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMutateGraphVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleShortestPathsVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleSuperstepVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleVertexWithWorkerContext.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/VerifyMessage.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertexResolver.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspUtils.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/EdgeListVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/HashMapVertex.java PRE-CREATION 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/IntIntNullIntVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/MutableVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/VertexResolver.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/Partition.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestBspBasic.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestJsonBase64Format.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestVertexTypes.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/examples/SimpleShortestPathVertexTest.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/lib/TestLongDoubleDoubleAdjacencyListVertexInputFormat.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/lib/TestTextDoubleDoubleAdjacencyListVertexInputFormat.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/utils/MockUtils.java 1226330 
bq.  
bq.  Diff: https://reviews.apache.org/r/3349/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  Passed unittests and pseudo-distributed MR tests.  Passed newly added unittests as well for EdgeListVertex.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Avery
bq.  
bq.


                
> Make EdgeListVertex the default vertex implementation, fix bugs related to EdgeListVertex.
> ------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-116
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-116
>             Project: Giraph
>          Issue Type: Improvement
>            Reporter: Avery Ching
>            Assignee: Avery Ching
>         Attachments: GIRAPH-116.patch
>
>
> I think this would best for new users as it is much more memory efficient than Vertex with respect to edges (list vs hash map).  We seem to be mostly iterating over the edges (as several others had pointed out in earlier JIRAs and emails), so this would provide early users with a more memory efficient implementation without performance loss.  If anyone disagrees, please voice your opinions.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Resolved] (GIRAPH-116) Make EdgeListVertex the default vertex implementation, fix bugs related to EdgeListVertex.

Posted by "Avery Ching (Resolved) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/GIRAPH-116?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Avery Ching resolved GIRAPH-116.
--------------------------------

    Resolution: Fixed

Thanks for the fast review Sebastian.  Hopefully new folks will be less irritated by high memory use.  =)
                
> Make EdgeListVertex the default vertex implementation, fix bugs related to EdgeListVertex.
> ------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-116
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-116
>             Project: Giraph
>          Issue Type: Improvement
>            Reporter: Avery Ching
>            Assignee: Avery Ching
>         Attachments: GIRAPH-116.patch
>
>
> I think this would best for new users as it is much more memory efficient than Vertex with respect to edges (list vs hash map).  We seem to be mostly iterating over the edges (as several others had pointed out in earlier JIRAs and emails), so this would provide early users with a more memory efficient implementation without performance loss.  If anyone disagrees, please voice your opinions.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (GIRAPH-116) Make EdgeListVertex the default vertex implementation, fix bugs related to EdgeListVertex.

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/GIRAPH-116?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13178498#comment-13178498 ] 

jiraposter@reviews.apache.org commented on GIRAPH-116:
------------------------------------------------------



bq.  On 2012-01-02 11:04:25, Sebastian Schelter wrote:
bq.  > Had a quick look over your changes and everything looked good. I think it's right to assume that most implementations will use static graphs and to offer EdgeListVertex as the default extension point for this. The only thing I don't like is the name change from MutableVertex to BasicVertex, I liked the former better because it sounds much more expressive to me.

Sebastian, I didn't change the name from MutableVertex to BasicVertex, sorry for the mixup.  I think MutableVertex is useful as well.  The only thing I did chance for for the addVertex() and addVertexReq() methods to take a BasicVertex rather than a MutableVertex to be a tad safer.  We still have the BasicVertex -> MutableVertex -> User vertex class hierarchy.


bq.  On 2012-01-02 11:04:25, Sebastian Schelter wrote:
bq.  > http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java, line 61
bq.  > <https://reviews.apache.org/r/3349/diff/2/?file=66112#file66112line61>
bq.  >
bq.  >     good thing to have the Iterable<> abstraction here

=)


- Avery


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3349/#review4168
-----------------------------------------------------------


On 2012-01-02 02:35:50, Avery Ching wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/3349/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2012-01-02 02:35:50)
bq.  
bq.  
bq.  Review request for giraph.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  * Changed Vertex.java to HashMapVertex.java.  This makes it less likely folks will use it as a default.  I have included comments that suggest EdgeListVertex for static graphs (most cases).
bq.  
bq.  * Found and fixed bugs in EdgeListVertex with the way that binarySearch was being used.  Added unittests to check for adding/getting/removing edges.
bq.  
bq.  * Changed classes that extend Vertex to extending EdgeListVertex instead.
bq.  
bq.  * Changed MutableVertex to BasicVertex for addVertex, addVertexReq to be a little safer
bq.  
bq.  * Tried to make sure that when a class that extends MutableVertex is instantiated that it also will call readFields() or initialize().  This fixed several bugs.
bq.  
bq.  * Changed the interface of BasicVertex#initialize from  public abstract void initialize(I vertexId, V vertexValue, Map<I, E> edges, List<M> messages) to 
bq.  initialize(I vertexId, V vertexValue, Map<I, E> edges, Iterable<M> messages) to better fit the recent changes to BasicVertex getting/setting messages with an Iterable.
bq.  
bq.  * Found and removed duplicated code from several MutableVertex extended classes for addVertexRequest, removeVertexRequest, addEdgeRequest and removeEdgeRequest.
bq.  
bq.  * Changed Vertex cast to BasicVertex cast in Partition and MockUtils.
bq.  
bq.  * There are some tabs --> spaces conversions done automatically from my Ecipse settings for the files I touched.
bq.  
bq.  
bq.  This addresses bug GIRAPH-116.
bq.      https://issues.apache.org/jira/browse/GIRAPH-116
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PageRankBenchmark.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PseudoRandomVertexInputFormat.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/CommunicationsInterface.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerCommunications.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCombinerVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleFailVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMsgVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMutateGraphVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleShortestPathsVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleSuperstepVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleVertexWithWorkerContext.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/VerifyMessage.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertexResolver.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspUtils.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/EdgeListVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/HashMapVertex.java PRE-CREATION 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/IntIntNullIntVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/MutableVertex.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/VertexResolver.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/Partition.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestBspBasic.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestJsonBase64Format.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestVertexTypes.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/examples/SimpleShortestPathVertexTest.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/lib/TestLongDoubleDoubleAdjacencyListVertexInputFormat.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/lib/TestTextDoubleDoubleAdjacencyListVertexInputFormat.java 1226330 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/utils/MockUtils.java 1226330 
bq.  
bq.  Diff: https://reviews.apache.org/r/3349/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  Passed unittests and pseudo-distributed MR tests.  Passed newly added unittests as well for EdgeListVertex.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Avery
bq.  
bq.


                
> Make EdgeListVertex the default vertex implementation, fix bugs related to EdgeListVertex.
> ------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-116
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-116
>             Project: Giraph
>          Issue Type: Improvement
>            Reporter: Avery Ching
>            Assignee: Avery Ching
>         Attachments: GIRAPH-116.patch
>
>
> I think this would best for new users as it is much more memory efficient than Vertex with respect to edges (list vs hash map).  We seem to be mostly iterating over the edges (as several others had pointed out in earlier JIRAs and emails), so this would provide early users with a more memory efficient implementation without performance loss.  If anyone disagrees, please voice your opinions.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (GIRAPH-116) Make EdgeListVertex the default vertex implementation, fix bugs related to EdgeListVertex.

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/GIRAPH-116?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13178506#comment-13178506 ] 

jiraposter@reviews.apache.org commented on GIRAPH-116:
------------------------------------------------------


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3349/
-----------------------------------------------------------

(Updated 2012-01-02 18:51:05.007864)


Review request for giraph.


Changes
-------

Sorry, forgot to include the new tests for giraph/trunk/src/test/java/org/apache/giraph/graph/TestEdgeListVertex.java.  I'll commit and if there is a problem, I can fix it later.  The tests pass.


Summary
-------

* Changed Vertex.java to HashMapVertex.java.  This makes it less likely folks will use it as a default.  I have included comments that suggest EdgeListVertex for static graphs (most cases).

* Found and fixed bugs in EdgeListVertex with the way that binarySearch was being used.  Added unittests to check for adding/getting/removing edges.

* Changed classes that extend Vertex to extending EdgeListVertex instead.

* Changed MutableVertex to BasicVertex for addVertex, addVertexReq to be a little safer

* Tried to make sure that when a class that extends MutableVertex is instantiated that it also will call readFields() or initialize().  This fixed several bugs.

* Changed the interface of BasicVertex#initialize from  public abstract void initialize(I vertexId, V vertexValue, Map<I, E> edges, List<M> messages) to 
initialize(I vertexId, V vertexValue, Map<I, E> edges, Iterable<M> messages) to better fit the recent changes to BasicVertex getting/setting messages with an Iterable.

* Found and removed duplicated code from several MutableVertex extended classes for addVertexRequest, removeVertexRequest, addEdgeRequest and removeEdgeRequest.

* Changed Vertex cast to BasicVertex cast in Partition and MockUtils.

* There are some tabs --> spaces conversions done automatically from my Ecipse settings for the files I touched.


This addresses bug GIRAPH-116.
    https://issues.apache.org/jira/browse/GIRAPH-116


Diffs (updated)
-----

  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PageRankBenchmark.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PseudoRandomVertexInputFormat.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/CommunicationsInterface.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerCommunications.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCombinerVertex.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleFailVertex.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMsgVertex.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMutateGraphVertex.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleShortestPathsVertex.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleSuperstepVertex.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleVertexWithWorkerContext.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/VerifyMessage.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertexResolver.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspUtils.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/EdgeListVertex.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/HashMapVertex.java PRE-CREATION 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/IntIntNullIntVertex.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/MutableVertex.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/VertexResolver.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/Partition.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestBspBasic.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestJsonBase64Format.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestVertexTypes.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/examples/SimpleShortestPathVertexTest.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/graph/TestEdgeListVertex.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/lib/TestLongDoubleDoubleAdjacencyListVertexInputFormat.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/lib/TestTextDoubleDoubleAdjacencyListVertexInputFormat.java 1226507 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/utils/MockUtils.java 1226507 

Diff: https://reviews.apache.org/r/3349/diff


Testing
-------

Passed unittests and pseudo-distributed MR tests.  Passed newly added unittests as well for EdgeListVertex.


Thanks,

Avery


                
> Make EdgeListVertex the default vertex implementation, fix bugs related to EdgeListVertex.
> ------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-116
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-116
>             Project: Giraph
>          Issue Type: Improvement
>            Reporter: Avery Ching
>            Assignee: Avery Ching
>         Attachments: GIRAPH-116.patch
>
>
> I think this would best for new users as it is much more memory efficient than Vertex with respect to edges (list vs hash map).  We seem to be mostly iterating over the edges (as several others had pointed out in earlier JIRAs and emails), so this would provide early users with a more memory efficient implementation without performance loss.  If anyone disagrees, please voice your opinions.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (GIRAPH-116) Make EdgeListVertex the default vertex implementation, fix bugs related to EdgeListVertex.

Posted by "Avery Ching (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/GIRAPH-116?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Avery Ching updated GIRAPH-116:
-------------------------------

    Summary: Make EdgeListVertex the default vertex implementation, fix bugs related to EdgeListVertex.  (was: Make EdgeListVertex the default vertex implementation)
    
> Make EdgeListVertex the default vertex implementation, fix bugs related to EdgeListVertex.
> ------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-116
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-116
>             Project: Giraph
>          Issue Type: Improvement
>            Reporter: Avery Ching
>            Assignee: Avery Ching
>
> I think this would best for new users as it is much more memory efficient than Vertex with respect to edges (list vs hash map).  We seem to be mostly iterating over the edges (as several others had pointed out in earlier JIRAs and emails), so this would provide early users with a more memory efficient implementation without performance loss.  If anyone disagrees, please voice your opinions.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (GIRAPH-116) Make EdgeListVertex the default vertex implementation, fix bugs related to EdgeListVertex.

Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/GIRAPH-116?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13178262#comment-13178262 ] 

jiraposter@reviews.apache.org commented on GIRAPH-116:
------------------------------------------------------


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3349/
-----------------------------------------------------------

Review request for giraph.


Summary
-------

* Changed Vertex.java to HashMapVertex.java.  This makes it less likely folks will use it as a default.  I have included comments that suggest EdgeListVertex for static graphs (most cases).

* Found and fixed bugs in EdgeListVertex with the way that binarySearch was being used.  Added unittests to check for adding/getting/removing edges.

* Changed classes that extend Vertex to extending EdgeListVertex instead.

* Changed MutableVertex to BasicVertex for addVertex, addVertexReq to be a little safer

* Tried to make sure that when a class that extends MutableVertex is instantiated that it also will call readFields() or initialize().  This fixed several bugs.

* Changed the interface of BasicVertex#initialize from  public abstract void initialize(I vertexId, V vertexValue, Map<I, E> edges, List<M> messages) to 
initialize(I vertexId, V vertexValue, Map<I, E> edges, Iterable<M> messages) to better fit the recent changes to BasicVertex getting/setting messages with an Iterable.

* Found and removed duplicated code from several MutableVertex extended classes for addVertexRequest, removeVertexRequest, addEdgeRequest and removeEdgeRequest.

* Changed Vertex cast to BasicVertex cast in Partition and MockUtils.

* There are some tabs --> spaces conversions done automatically from my Ecipse settings for the files I touched.


This addresses bug GIRAPH-116.
    https://issues.apache.org/jira/browse/GIRAPH-116


Diffs
-----

  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PageRankBenchmark.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PseudoRandomVertexInputFormat.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/CommunicationsInterface.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerCommunications.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCombinerVertex.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleFailVertex.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMsgVertex.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleMutateGraphVertex.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleShortestPathsVertex.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleSuperstepVertex.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleVertexWithWorkerContext.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/VerifyMessage.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertex.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BasicVertexResolver.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspUtils.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/EdgeListVertex.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/HashMapVertex.java PRE-CREATION 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/IntIntNullIntVertex.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/LongDoubleFloatDoubleVertex.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/MutableVertex.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/VertexResolver.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/Partition.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestBspBasic.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestJsonBase64Format.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestVertexTypes.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/examples/SimpleShortestPathVertexTest.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/lib/TestLongDoubleDoubleAdjacencyListVertexInputFormat.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/lib/TestTextDoubleDoubleAdjacencyListVertexInputFormat.java 1226330 
  http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/utils/MockUtils.java 1226330 

Diff: https://reviews.apache.org/r/3349/diff


Testing
-------

Passed unittests and pseudo-distributed MR tests.  Passed newly added unittests as well for EdgeListVertex.


Thanks,

Avery


                
> Make EdgeListVertex the default vertex implementation, fix bugs related to EdgeListVertex.
> ------------------------------------------------------------------------------------------
>
>                 Key: GIRAPH-116
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-116
>             Project: Giraph
>          Issue Type: Improvement
>            Reporter: Avery Ching
>            Assignee: Avery Ching
>         Attachments: GIRAPH-116.patch
>
>
> I think this would best for new users as it is much more memory efficient than Vertex with respect to edges (list vs hash map).  We seem to be mostly iterating over the edges (as several others had pointed out in earlier JIRAs and emails), so this would provide early users with a more memory efficient implementation without performance loss.  If anyone disagrees, please voice your opinions.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira