----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14527/#review26775 -----------------------------------------------------------
Avery, this is awesome! It will improve the performance of vertex input, and remove the need for preprocessing data in some applications. CHANGELOG <https://reviews.apache.org/r/14527/#comment52104> Please revert giraph-core/src/main/java/org/apache/giraph/comm/SendCache.java <https://reviews.apache.org/r/14527/#comment52105> Partition cache type? giraph-core/src/main/java/org/apache/giraph/comm/SendVertexIdDataCache.java <https://reviews.apache.org/r/14527/#comment52117> It's not clean that all implementations have to care about updating the size whenever they modify data in the super class. Although I'm not sure how to do it better, so if you don't either I'm fine with it staying this way. giraph-core/src/main/java/org/apache/giraph/comm/requests/SendWorkerVerticesRequest.java <https://reviews.apache.org/r/14527/#comment52106> Add comment describing where 13 comes from giraph-core/src/main/java/org/apache/giraph/conf/GiraphClasses.java <https://reviews.apache.org/r/14527/#comment52114> vertexValueCombinerClass giraph-core/src/main/java/org/apache/giraph/conf/GiraphClasses.java <https://reviews.apache.org/r/14527/#comment52115> getVertexValueCombinerClass giraph-core/src/main/java/org/apache/giraph/job/GiraphConfigurationValidator.java <https://reviews.apache.org/r/14527/#comment52107> Call verifyVertexCombinerGenericTypes(); giraph-core/src/main/java/org/apache/giraph/job/GiraphConfigurationValidator.java <https://reviews.apache.org/r/14527/#comment52108> Leftover from VertexCombiner - we have just vertex value param now giraph-core/src/main/java/org/apache/giraph/partition/ByteArrayPartition.java <https://reviews.apache.org/r/14527/#comment52109> I think this is more inefficient than it needs to be. How about we call putIfAbsent, and then only if that doesn't succeed we synchronize? And when we synchronize, maybe we can just synchronize on the vertex id object from the map instead of the whole partition? giraph-core/src/main/java/org/apache/giraph/partition/ByteArrayPartition.java <https://reviews.apache.org/r/14527/#comment52120> Same question as above giraph-core/src/main/java/org/apache/giraph/partition/DiskBackedPartitionStore.java <https://reviews.apache.org/r/14527/#comment52112> Why this needed to be modified? giraph-core/src/main/java/org/apache/giraph/partition/BasicPartition.java <https://reviews.apache.org/r/14527/#comment52122> Why don't we use the return value of putOrCombine to decide whether to release or not? - Maja Kabiljo On Oct. 8, 2013, 6:10 p.m., Avery Ching wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/14527/ > ----------------------------------------------------------- > > (Updated Oct. 8, 2013, 6:10 p.m.) > > > Review request for giraph. > > > Bugs: GIRAPH-775 > https://issues.apache.org/jira/browse/GIRAPH-775 > > > Repository: giraph-git > > > Description > ------- > > * Implements vertex value combining at load time. > * Vertices are sent to destination workers via a immediately serialized > vertex cache instead of a cached partition > * Fixed a bug where a vertex input format's edges would be overwritten by an > edge input format's edges > * Changed the name of combiner to message combiner and vertex value combiner > to be more explicit - hence the large number of changes. > * Added new IntIntNullTextVertexInputFormat that takes in id, value, edges > > Added 2 new unittests: > * Testing vertex value combiner > * Testing mixed vertex and edge input formats with overlapping edges > > > Diffs > ----- > > CHANGELOG f960d0904f1bfaea8b2b7cac60b3e37e7bca7ba2 > > giraph-core/src/main/java/org/apache/giraph/benchmark/PageRankBenchmark.java > acc1c461abc04d9a1e7d1128e6fe3f5fb06ca4e9 > > giraph-core/src/main/java/org/apache/giraph/benchmark/ShortestPathsBenchmark.java > 0dd4529332f0fe7223be194c64337f4aa95a75b6 > > giraph-core/src/main/java/org/apache/giraph/benchmark/WeightedPageRankBenchmark.java > 2077674fd104c7275e6d086b26faa7039b7091e1 > giraph-core/src/main/java/org/apache/giraph/combiner/Combiner.java > 7830ffff079c945eb8b0258a733ff72dc426b797 > giraph-core/src/main/java/org/apache/giraph/combiner/DoubleSumCombiner.java > 8da4ba77b6f68d8f901b9aa4f2ecc439eec5434d > giraph-core/src/main/java/org/apache/giraph/combiner/FloatSumCombiner.java > d89879150efa7ee0df9845f26dc9390a9b632171 > > giraph-core/src/main/java/org/apache/giraph/combiner/MinimumDoubleCombiner.java > 0a85d64c388582e1f7d25c57da2772ada51f9ea5 > > giraph-core/src/main/java/org/apache/giraph/combiner/MinimumIntCombiner.java > fcef58eaec13c22ffa01c34d0a3ec29ab0fee53d > giraph-core/src/main/java/org/apache/giraph/combiner/SimpleSumCombiner.java > 2a11d2f06be9365ec8ef6dae0759a9e47a4f08c0 > giraph-core/src/main/java/org/apache/giraph/comm/SendCache.java > 30c07ee957b66b9c11495cabf3719f94e445acf1 > giraph-core/src/main/java/org/apache/giraph/comm/SendEdgeCache.java > 5513da2a6a4bed63ae0915fb257e830b6c077fa5 > giraph-core/src/main/java/org/apache/giraph/comm/SendMessageCache.java > 8df0dda628e1753896eaa0ed96a5383318056612 > giraph-core/src/main/java/org/apache/giraph/comm/SendPartitionCache.java > 524c9f1cf6a605d9e06b216b980b8e21712d7af5 > giraph-core/src/main/java/org/apache/giraph/comm/SendVertexIdDataCache.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/comm/WorkerClientRequestProcessor.java > 9bdf9cae20b47b5063b343e1a9d42babfd37df96 > > giraph-core/src/main/java/org/apache/giraph/comm/messages/InMemoryMessageStoreFactory.java > 0cdfb733014496b84f26fceb35cb9f509ac1eac7 > > giraph-core/src/main/java/org/apache/giraph/comm/messages/MessageStoreFactory.java > 254afd404716ecb4793a2f22dc336ddf53e20242 > > giraph-core/src/main/java/org/apache/giraph/comm/messages/OneMessagePerVertexStore.java > 4f150daeaf639a6f4de6ba6b45d47f514e7f38cc > > giraph-core/src/main/java/org/apache/giraph/comm/messages/primitives/IntByteArrayMessageStore.java > cdab2e0c647f7d4a578db016ea1c27d362b6c8a7 > > giraph-core/src/main/java/org/apache/giraph/comm/messages/primitives/IntFloatMessageStore.java > a193fb97ce6e0a86fbfd61dee6dd3d8249c2ad8b > > giraph-core/src/main/java/org/apache/giraph/comm/messages/primitives/LongByteArrayMessageStore.java > 3272ced26b66da56da7413430dedadaeadbafafd > > giraph-core/src/main/java/org/apache/giraph/comm/messages/primitives/LongDoubleMessageStore.java > 96ed5b4981c670f0534d22bce406b4d39796d4e9 > > giraph-core/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClient.java > 28f365693ce3e8701fa3cb72ea8683733ea3691e > > giraph-core/src/main/java/org/apache/giraph/comm/netty/NettyWorkerClientRequestProcessor.java > 34a3d1ff86a4f73ecefad4891a5d8d04368e653b > > giraph-core/src/main/java/org/apache/giraph/comm/netty/NettyWorkerServer.java > 3473de1c57c05333c5731775ef85a19c95717a28 > > giraph-core/src/main/java/org/apache/giraph/comm/netty/handler/RequestEncoder.java > 83b408e3102696a859acb5c0286b541f44bacddf > giraph-core/src/main/java/org/apache/giraph/comm/requests/RequestType.java > a1dcece230d63f7b3bed583ea3bc437b481eab03 > > giraph-core/src/main/java/org/apache/giraph/comm/requests/SendWorkerVerticesRequest.java > PRE-CREATION > giraph-core/src/main/java/org/apache/giraph/conf/GiraphClasses.java > f97446fbc3c46365338fb1ef33e203fa2266eda6 > giraph-core/src/main/java/org/apache/giraph/conf/GiraphConfiguration.java > 15ff861ca062b8981b6dd2a385b4e64f7653b968 > giraph-core/src/main/java/org/apache/giraph/conf/GiraphConstants.java > 4dadd29d8f82ccc7b918d9e190c7f439c4da7134 > > giraph-core/src/main/java/org/apache/giraph/conf/ImmutableClassesGiraphConfiguration.java > 435dfa5679e6c4738d649df0cd02f1387950eab8 > giraph-core/src/main/java/org/apache/giraph/edge/EdgeStore.java > 23df689121944d834803ffcba47cb5f61cb74451 > giraph-core/src/main/java/org/apache/giraph/graph/ComputeCallable.java > 77d9f5ebc5a283794c1e26b0d5f0a723e018f766 > > giraph-core/src/main/java/org/apache/giraph/graph/DefaultVertexValueCombiner.java > PRE-CREATION > giraph-core/src/main/java/org/apache/giraph/graph/VertexValueCombiner.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/io/formats/IntIntNullTextVertexInputFormat.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/io/formats/TextVertexValueInputFormat.java > 6a795a8fe640321b34ef689315436fa6c73132bd > > giraph-core/src/main/java/org/apache/giraph/job/GiraphConfigurationValidator.java > 5b870c54e92a7ece943bef04e3264e5703cb815a > giraph-core/src/main/java/org/apache/giraph/jython/JythonJob.java > 6b2eedf8bf82bf13e10eb20e2d98b8a960348844 > giraph-core/src/main/java/org/apache/giraph/master/MasterCompute.java > cf7356cf9f98ef120b7d6bd14af421e9bceeacf2 > giraph-core/src/main/java/org/apache/giraph/master/SuperstepClasses.java > 7a7df052e8b7b937b14d032875f5301a02ea9a8f > giraph-core/src/main/java/org/apache/giraph/partition/BasicPartition.java > f2b855249c6a126d2e042048489e71baa6d65ea4 > > giraph-core/src/main/java/org/apache/giraph/partition/ByteArrayPartition.java > 6eaa6d76efac80c53251ad1037f1c44ed6c4d933 > > giraph-core/src/main/java/org/apache/giraph/partition/DiskBackedPartitionStore.java > 110ce9d5520aa76763d925615aea34fafca1dc52 > giraph-core/src/main/java/org/apache/giraph/partition/Partition.java > b6b9551f20fbeb325b9033ea9fe79accc9f4e351 > giraph-core/src/main/java/org/apache/giraph/partition/PartitionStore.java > 763397e0f7e6b7d09c51b1679eed3097d2f89e01 > giraph-core/src/main/java/org/apache/giraph/partition/SimplePartition.java > 0c1b40439a8891629b60aace4d89997ade42d1b5 > > giraph-core/src/main/java/org/apache/giraph/partition/SimplePartitionStore.java > ae17aac19001ccacc147287cc7a303d8cc17c91f > > giraph-core/src/main/java/org/apache/giraph/utils/ByteArrayVertexIdMessages.java > 7e2b73b08be2a371b1dfe9118f5b1ca6f75b0d5e > giraph-core/src/main/java/org/apache/giraph/utils/ConfigurationUtils.java > 4bc4f4d5848590902894d35aad8c325177851604 > giraph-core/src/main/java/org/apache/giraph/utils/VertexIterator.java > PRE-CREATION > giraph-core/src/main/java/org/apache/giraph/utils/WritableUtils.java > 9163c08c6e4d089ec467f8a223efdaabef2bb995 > giraph-core/src/main/java/org/apache/giraph/worker/BspServiceWorker.java > 112b76d03a234246ec9971f7f70be4b550ae5222 > giraph-core/src/test/java/org/apache/giraph/comm/RequestTest.java > 115c10839a03a01d15a53ccf3e90caeb0db498ee > > giraph-core/src/test/java/org/apache/giraph/comm/messages/TestIntFloatPrimitiveMessageStores.java > a8f6f70cdb59edf55e2eb441564e9c7ff5376ae5 > > giraph-core/src/test/java/org/apache/giraph/comm/messages/TestLongDoublePrimitiveMessageStores.java > 065926092bc886d680ad07849ad1b614e82b5bab > giraph-core/src/test/java/org/apache/giraph/io/TestEdgeInput.java > 45d946fbeeadc71213801ac33c20686bb782a109 > > giraph-core/src/test/java/org/apache/giraph/master/TestComputationCombinerTypes.java > b62775f1c7b3f561b5e7b05bb3ffb66ac42493c2 > giraph-core/src/test/java/org/apache/giraph/master/TestSwitchClasses.java > 6b0ed35391942f472fca43eeb773ad59715ffa7f > > giraph-core/src/test/java/org/apache/giraph/partition/TestPartitionStores.java > 0e68b56adac6a838cd314a169fdd9cdef1ca059f > giraph-examples/src/test/java/org/apache/giraph/TestBspBasic.java > 115da7e7a0e7c8c0ada15db5dc5b9e229e1d4290 > > giraph-examples/src/test/java/org/apache/giraph/examples/ConnectedComponentsComputationTest.java > 7d326da9d5aa338aed87f9fddf5b292d17489329 > > giraph-examples/src/test/java/org/apache/giraph/examples/ConnectedComponentsComputationTestInMemory.java > dbcd569c216939a3fdd7bdddacf5871f500b8f28 > > giraph-examples/src/test/java/org/apache/giraph/examples/MinimumIntCombinerTest.java > 434c7561806fc099f077b526cff9ded1bdc3fa11 > > giraph-examples/src/test/java/org/apache/giraph/examples/TryMultiIpcBindingPortsTest.java > 1323ff6dda458c9589ab8e3e7a1173c7881a49c1 > > giraph-examples/src/test/java/org/apache/giraph/vertex/TestComputationTypes.java > 4f74fcb0ff835ee0fbfbf2cc822083c0bfbd0814 > > giraph-hive/src/main/java/org/apache/giraph/hive/input/vertex/examples/HiveIntIntNullVertex.java > PRE-CREATION > > giraph-hive/src/main/java/org/apache/giraph/hive/jython/HiveJythonUtils.java > 334f3828fedc5c077b9492e6f1a31a8e515082e4 > > giraph-hive/src/test/java/org/apache/giraph/hive/input/HiveVertexInputTest.java > c75652cc2f979736e1b8c72f88717f4ab0fe2e31 > > giraph-hive/src/test/resources/org/apache/giraph/jython/count-edges-launcher.py > a87f0309dce3f54332b76ff5239dffa3758c4224 > src/site/xdoc/quick_start.xml 5df6555e5d93bfae173b42bee4bbeaaf36366d1c > > Diff: https://reviews.apache.org/r/14527/diff/ > > > Testing > ------- > > Passes all new unittests > mvn clean verify > > > Thanks, > > Avery Ching > >
