[ 
https://issues.apache.org/jira/browse/GIRAPH-37?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13271550#comment-13271550
 ] 

jirapos...@reviews.apache.org commented on GIRAPH-37:
-----------------------------------------------------



bq.  On 2012-05-09 10:10:46, Sebastian Schelter wrote:
bq.  > 
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java,
 line 1465
bq.  > 
<https://reviews.apache.org/r/5074/diff/2/?file=108120#file108120line1465>
bq.  >
bq.  >     I don't like it that a collection is changed outside of the class 
that owns it. 
bq.  >     
bq.  >     This makes code hard to read and debug. We should rather introduce a 
method for this in the class that owns this map to have all mutations in one 
place.

Good point, it's a little heard to understand.  Since this is a Map, we can do 
as you suggested, keep it in a class and then add a method to do the clear().  
We can even add calls to do the methods that iterate over the map as well to 
not have to do any synchronization outside of the map.  I'll do this for all 
our synchronized objects in the next patch if that's okay with you (the current 
code does this as well).  It will be a somewhat medium sized change.


- Avery


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


On 2012-05-09 09:22:36, Avery Ching wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/5074/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2012-05-09 09:22:36)
bq.  
bq.  
bq.  Review request for giraph.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  * Implemented a request/response protocol with netty as a NettyClient and 
NettyServer.  There is a NettyClientWorker and NettyClientServer that 
implements WorkerClient and WorkerServer, respectively.  Netty is a lot faster 
since it's non-blocking and we can interleave computation and communication as 
opposed to Hadoop RPC (blocking).
bq.  * The netty server implementation uses concurrent hash maps to improved 
concurrency instead of synchronized blocks around maps.
bq.  * By default netty is used, but Hadoop RPC can be used with 
-Dgiraph.useNetty=false
bq.  * Changed the class hierarchy of ServerInterface to WorkerClientServer 
(WorkerClient and WorkerServer) to support a request/response protocol instead 
of just RPC
bq.  * In netty, the messages/mutations are gathered by partition and send out 
as a partition's worth of messages/mutations
bq.  * Added two new test classes (RequestTest.java and ConnectionTest.java) to 
test all requests and check netty connections.
bq.  * PageRankBenchmark uses EdgeListVertex as a default
bq.  
bq.  
bq.  This addresses bug GIRAPH-37.
bq.      https://issues.apache.org/jira/browse/GIRAPH-37
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/utils/MockUtils.java
 1332888 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/examples/SimpleShortestPathVertexTest.java
 1332888 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/comm/RequestTest.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/WorkerInfo.java
 1332888 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestJsonBase64Format.java
 1332888 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestManualCheckpoint.java
 1332888 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/comm/ConnectionTest.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/GiraphJob.java
 1332888 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/GraphState.java
 1332888 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/VertexMutations.java
 1332888 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/WritableRequest.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspService.java
 1332888 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java
 1332888 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerClientServer.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerCommunications.java
 1332888 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerServer.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/WorkerClient.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/ServerInterface.java
 1332888 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/ServerData.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/SendVertexRequest.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/SendPartitionMutationsRequest.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/SendPartitionMessagesRequest.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/SendMutationsCache.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/SendMessageCache.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestServerHandler.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/ResponseClientHandler.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestRegistry.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestEncoder.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/RequestDecoder.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerServer.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerClientServer.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyWorkerClient.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyServer.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/NettyClient.java
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java
 1332888 
bq.    http://svn.apache.org/repos/asf/incubator/giraph/trunk/pom.xml 1332888 
bq.    
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PageRankBenchmark.java
 1332888 
bq.  
bq.  Diff: https://reviews.apache.org/r/5074/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  'mvn verify' passes.  I ran several test runs to gather performance 
results.  Here is a simple example:
bq.  
bq.  Hadoop RPC:
bq.  hadoop jar ~/giraph-0.2-SNAPSHOT-jar-with-dependencies.jar 
org.apache.giraph.benchmark.PageRankBenchmark -Dgiraph.useNetty=false -w 5 -V 
5000000 -s 5 -e 2 -v
bq.  
bq.  12/05/09 01:59:56 INFO mapred.JobClient:   Giraph Timers
bq.  12/05/09 01:59:56 INFO mapred.JobClient:     Total (milliseconds)=167722
bq.  12/05/09 01:59:56 INFO mapred.JobClient:     Superstep 3 
(milliseconds)=24775
bq.  12/05/09 01:59:56 INFO mapred.JobClient:     Setup (milliseconds)=2930
bq.  12/05/09 01:59:56 INFO mapred.JobClient:     Shutdown (milliseconds)=181
bq.  12/05/09 01:59:56 INFO mapred.JobClient:     Vertex input superstep 
(milliseconds)=51025
bq.  12/05/09 01:59:56 INFO mapred.JobClient:     Superstep 0 
(milliseconds)=21543
bq.  12/05/09 01:59:56 INFO mapred.JobClient:     Superstep 4 
(milliseconds)=19858
bq.  12/05/09 01:59:56 INFO mapred.JobClient:     Superstep 5 
(milliseconds)=2844
bq.  12/05/09 01:59:56 INFO mapred.JobClient:     Superstep 2 
(milliseconds)=24507
bq.  12/05/09 01:59:56 INFO mapred.JobClient:     Superstep 1 
(milliseconds)=20054
bq.  
bq.  Netty:
bq.  hadoop jar ~/giraph-0.2-SNAPSHOT-jar-with-dependencies.jar 
org.apache.giraph.benchmark.PageRankBenchmark -Dgiraph.useNetty=true -w 5 -V 
5000000 -s 5 -e 2 -v
bq.  
bq.  12/05/09 02:06:10 INFO mapred.JobClient:   Giraph Timers
bq.  12/05/09 02:06:10 INFO mapred.JobClient:     Total (milliseconds)=57795
bq.  12/05/09 02:06:10 INFO mapred.JobClient:     Superstep 3 
(milliseconds)=7636
bq.  12/05/09 02:06:10 INFO mapred.JobClient:     Setup (milliseconds)=3574
bq.  12/05/09 02:06:10 INFO mapred.JobClient:     Shutdown (milliseconds)=232
bq.  12/05/09 02:06:10 INFO mapred.JobClient:     Vertex input superstep 
(milliseconds)=13393
bq.  12/05/09 02:06:10 INFO mapred.JobClient:     Superstep 0 
(milliseconds)=5610
bq.  12/05/09 02:06:10 INFO mapred.JobClient:     Superstep 4 
(milliseconds)=8473
bq.  12/05/09 02:06:10 INFO mapred.JobClient:     Superstep 5 
(milliseconds)=1844
bq.  12/05/09 02:06:10 INFO mapred.JobClient:     Superstep 2 
(milliseconds)=7418
bq.  12/05/09 02:06:10 INFO mapred.JobClient:     Superstep 1 
(milliseconds)=9612
bq.  
bq.  These were some median runs. The overall runtime improved from 167722 -> 
57795 with Netty (2.9x faster).  Loading the vertices improved from 51025 -bq.  
13393 (3.8x faster).  More results coming tomorrow, but for bigger runs, the 
improvement is likely to be even more than 3x.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Avery
bq.  
bq.


                
> Implement Netty-backed rpc solution
> -----------------------------------
>
>                 Key: GIRAPH-37
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-37
>             Project: Giraph
>          Issue Type: New Feature
>            Reporter: Jakob Homan
>            Assignee: Jakob Homan
>         Attachments: GIRAPH-37-wip.patch, GIRAPH-37.patch
>
>
> GIRAPH-12 considered replacing the current Hadoop based rpc method with 
> Netty, but didn't went in another direction. I think there is still value in 
> this approach, and will also look at Finagle.

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

        

Reply via email to