----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7206/#review11788 -----------------------------------------------------------
Avery, this seems like a similar situation as GIRAPH-262/GIRAPH-289, where we can break out a separate JIRA for subclassing Configuration. GIRAPH-329 would depend on that one. http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/GiraphConfiguration.java <https://reviews.apache.org/r/7206/#comment25324> Wouldn't it be good to split this out into another JIRA ("Specialize org.apache.hadoop.conf.Configuration to GiraphConfiguration")? http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PageRankBenchmark.java <https://reviews.apache.org/r/7206/#comment25325> I like this improvement! Much cleaner and easier to read, and less intimidating for beginners. - Eugene Koontz On Sept. 21, 2012, 8:12 a.m., Avery Ching wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7206/ > ----------------------------------------------------------- > > (Updated Sept. 21, 2012, 8:12 a.m.) > > > Review request for giraph. > > > Description > ------- > > Major changes: > - Configuration have been deprecated in favor of GiraphConfiguration for easy > get/set methods. Then I have ImmutableClassesGiraphConfiguration that will > generate the classes as final variables to be thread-safe. This is a big > improvement over Configuration#getClass(), which is very slow and makes > deserialization very slow. This represents the bulk of the changes as we > have to change code everywhere. All configuration variables have moved from > GiraphJob to GiraphConfiguration. Note that I didn't fix the vertex input / > output APIs to use ImmutableClassesGiraphConfiguration and just generate it > there for now. That can be done in a later change. > - There is a new ImmutableClassesGiraphConfigurable that replaces > Configurable and our ReflectionUtils understands this to make sure to > newInstance an object and set its ImmutableClassesGiraphConfiguration if it > is ImmutableClassesGiraphConfigurable. > - Upgraded/downgraded log messages to trace/debug to make debug more usable. > I'm able to run most jobs in debug with about a 10% loss in performance, not > too bad. Debug is now quite useful to figuring out performance issues, > general problems. > - Added an optimization to SimpleMessageStore#addPartitionMessages to avoid > getting the partition map for every vertex. This was one of the bottlenecks. > - Added client and server ExecutionHandlers to Netty to avoid doing the > business logic with I/O threads (This used to clog up the pipe) > - Added WrappedAdaptiveReceiveBufferSizePredictorFactory to debug/predict the > size of the incoming messages and provide better performance tuning on the > server instead of receiving a bunch of small messages. > > Minor changes: > - Added BspUtilsTest to try various ways of testing class creation, > performance and correctness. > - Upgrade junit from 4.0 -> 4.8 to allow getting the test name (See > BspUtilsTest.java). > > > This addresses bug GIRAPH-329. > https://issues.apache.org/jira/browse/GIRAPH-329 > > > Diffs > ----- > > http://svn.apache.org/repos/asf/giraph/trunk/pom.xml 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/GiraphConfiguration.java > PRE-CREATION > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/GiraphRunner.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/ImmutableClassesGiraphConfigurable.java > PRE-CREATION > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/ImmutableClassesGiraphConfiguration.java > PRE-CREATION > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/benchmark/EdgeListVertexPageRankBenchmark.java > PRE-CREATION > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/benchmark/PageRankBenchmark.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/benchmark/RandomMessageBenchmark.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/benchmark/ShortestPathsBenchmark.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/bsp/BspInputFormat.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/BasicRPCCommunications.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/RPCCommunications.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/SendMessageCache.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/ServerData.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/messages/DiskBackedMessageStore.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/messages/SequentialFileMessageStore.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/messages/SimpleMessageStore.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/netty/ByteCounter.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/netty/NettyClient.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/netty/NettyMasterClient.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/netty/NettyMasterClientServer.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/netty/NettyMasterServer.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/netty/NettyServer.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClient.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClientServer.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/netty/NettyWorkerServer.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/netty/WrappedAdaptiveReceiveBufferSizePredictorFactory.java > PRE-CREATION > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/netty/handler/AddressRequestIdGenerator.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/netty/handler/RequestDecoder.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/netty/handler/RequestEncoder.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/netty/handler/RequestInfo.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/netty/handler/RequestServerHandler.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/netty/handler/ResponseClientHandler.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/netty/handler/WorkerRequestReservedMap.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/requests/SendPartitionMessagesRequest.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/requests/SendPartitionMutationsRequest.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/requests/SendVertexRequest.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/comm/requests/WritableRequest.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/examples/GeneratedVertexReader.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/examples/SimplePageRankVertex.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleSuperstepVertex.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleVertexWithWorkerContext.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspService.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceMaster.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspServiceWorker.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/BspUtils.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/EdgeListVertex.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/GiraphJob.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/GraphMapper.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/HashMapVertex.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/MasterCompute.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/MasterThread.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/MutableVertex.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/SimpleMutableVertex.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/SimpleVertex.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/Vertex.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/VertexMutations.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/VertexResolver.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/BasicPartitionOwner.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/DiskBackedPartitionStore.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/GraphPartitionerFactory.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/HashMasterPartitioner.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/HashPartitionerFactory.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/HashRangePartitionerFactory.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/Partition.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/RangePartitionOwner.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/RangeSplitHint.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/graph/partition/SimplePartitionStore.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/integration/SuperstepHashPartitionerFactory.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/io/JsonBase64VertexInputFormat.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/io/PseudoRandomVertexInputFormat.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/io/TextVertexInputFormat.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/utils/FakeTime.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/utils/InternalVertexRunner.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/utils/ReflectionUtils.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/utils/SystemTime.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/utils/Time.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/utils/WritableUtils.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/main/java/org/apache/giraph/zk/ZooKeeperManager.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/BspCase.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/TestAggregatorsHandling.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/TestAutoCheckpoint.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/TestBspBasic.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/TestGraphPartitioner.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/TestJsonBase64Format.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/TestManualCheckpoint.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/TestNotEnoughMapTasks.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/TestVertexTypes.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/aggregators/TestDoubleAggregators.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/aggregators/TestFloatAggregators.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/ConnectionTest.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/RPCCommunicationsTest.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/RequestFailureTest.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/RequestTest.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/comm/TestMessageStores.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/examples/SimpleShortestPathsVertexTest.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/examples/TryMultiRpcBindingPortsTest.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/graph/TestEdgeListVertex.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/graph/partition/TestPartitionStores.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/io/TestLongDoubleDoubleAdjacencyListVertexInputFormat.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/io/TestTextDoubleDoubleAdjacencyListVertexInputFormat.java > 1388358 > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/org/apache/giraph/utils/BspUtilsTest.java > PRE-CREATION > > http://svn.apache.org/repos/asf/giraph/trunk/src/test/java/zk/TestZooKeeperManager.java > 1388358 > > Diff: https://reviews.apache.org/r/7206/diff/ > > > Testing > ------- > > Passed all unittests, ran PageRankBenchmark many times on a real cluster. > See results in > https://issues.apache.org/jira/secure/attachment/12546012/GIRAPH-329.patch . > > > Thanks, > > Avery Ching > >
