----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12377/#review22952 -----------------------------------------------------------
giraph-core/src/main/java/org/apache/giraph/comm/netty/handler/LZ4RequestCompressor.java <https://reviews.apache.org/r/12377/#comment46736> Let's reuse UnsafeByteArrayOutputStream:SIZE_OF_INT / UnsafeByteArrayOutputStream:SIZE_OF_LONG. You probably want to move those constants to another place like Sizes or something. giraph-core/src/main/java/org/apache/giraph/comm/netty/handler/LZ4RequestCompressor.java <https://reviews.apache.org/r/12377/#comment46738> Just curious, is there some on-the-fly decompressor, or do we have to do it all at once? giraph-core/src/main/java/org/apache/giraph/comm/netty/handler/RequestCompressor.java <https://reviews.apache.org/r/12377/#comment46737> Make this an interface. Also rename to RequestCodec (since it's also a decompressor)? giraph-core/src/main/java/org/apache/giraph/comm/netty/handler/RequestEncoder.java <https://reviews.apache.org/r/12377/#comment46739> SIZE_OF_INT giraph-core/src/main/java/org/apache/giraph/conf/ImmutableClassesGiraphConfiguration.java <https://reviews.apache.org/r/12377/#comment46740> pass in this as well to allow configurable? giraph-core/src/main/java/org/apache/giraph/metrics/GiraphMetrics.java <https://reviews.apache.org/r/12377/#comment46741> ? giraph-core/src/main/java/org/apache/giraph/metrics/MetricNames.java <https://reviews.apache.org/r/12377/#comment46742> cool, nice to have histograms tracking it :) pom.xml <https://reviews.apache.org/r/12377/#comment46735> nit: put a constant above in <properties>, we should move all the versions up there. - Nitay Joffe On July 10, 2013, 12:16 a.m., Maja Kabiljo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12377/ > ----------------------------------------------------------- > > (Updated July 10, 2013, 12:16 a.m.) > > > Review request for giraph. > > > Bugs: GIRAPH-713 > https://issues.apache.org/jira/browse/GIRAPH-713 > > > Repository: giraph-git > > > Description > ------- > > In some cases, network is much slower than all the computation stuff we do, > and we could benefit from compressing requests. > > I am using LZ4 compression. Tried out two versions of Snappy and some java > ones, and LZ4 had the least CPU overhead, and about the same speed as Snappy. > It's also easy to plug in your own compressor. In cases when we are not > bounded by network, CPU overhead of using compression is about 10% (testing > with PageRankBenchmark) and time stays about the same. Depending on how slow > your network is and how good your data compresses this can lead to big time > savings. Also added compression to metrics, from one of the pagerank > iterations: > https://gist.github.com/majakabiljo/5962269 > > > Diffs > ----- > > giraph-core/pom.xml cab0157 > > giraph-core/src/main/java/org/apache/giraph/comm/netty/handler/LZ4RequestCompressor.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/comm/netty/handler/RequestCompressor.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/comm/netty/handler/RequestDecoder.java > 6eb6549 > > giraph-core/src/main/java/org/apache/giraph/comm/netty/handler/RequestEncoder.java > 83b408e > giraph-core/src/main/java/org/apache/giraph/conf/GiraphClasses.java 6655834 > giraph-core/src/main/java/org/apache/giraph/conf/GiraphConstants.java > c4cc96f > > giraph-core/src/main/java/org/apache/giraph/conf/ImmutableClassesGiraphConfiguration.java > ed63192 > giraph-core/src/main/java/org/apache/giraph/metrics/GiraphMetrics.java > 7d980ea > giraph-core/src/main/java/org/apache/giraph/metrics/MetricNames.java > cc237ac > > giraph-core/src/test/java/org/apache/giraph/comm/LZ4RequestCompressorTest.java > PRE-CREATION > pom.xml 72fe6f2 > > Diff: https://reviews.apache.org/r/12377/diff/ > > > Testing > ------- > > mvn clean verify > PageRank on cluster > Added test for compressor > Verified that NoOpCompressor has the same performance as before > > > Thanks, > > Maja Kabiljo > >
