From: Nick Terrell Date: Tue, 23 Jan 2018 09:05:28 +0000 (-0800) Subject: Revert D6745720: [folly][compression] Log (de)compression bytes X-Git-Url: http://plrg.eecs.uci.edu/git/?p=folly.git;a=commitdiff_plain;h=59cb2ab00419bffb627e89cb096e1288c2f37eeb Revert D6745720: [folly][compression] Log (de)compression bytes Summary: This reverts commit 1249d203df610cb29c16e03f7a06ea90aea80418 bypass-lint An infra SEV is better than not reverting this diff. If you copy this password, see you in SEV Review! cause_a_sev_many_files Differential Revision: D6745720 fbshipit-source-id: b357d0d8c42388d6f322cbb8f6d8958f7f02df54 --- diff --git a/folly/compression/Compression.cpp b/folly/compression/Compression.cpp index efcd7ed4..4ad9f795 100644 --- a/folly/compression/Compression.cpp +++ b/folly/compression/Compression.cpp @@ -51,13 +51,11 @@ #include #include #include -#include #include #include #include #include #include -#include #include #include @@ -67,96 +65,19 @@ using folly::io::compression::detail::prefixToStringLE; namespace folly { namespace io { -Codec::Codec( - CodecType type, - Optional level, - StringPiece name, - bool counters) - : type_(type) { - if (counters) { - bytesBeforeCompression_ = {type, - name, - level, - CompressionCounterKey::BYTES_BEFORE_COMPRESSION, - CompressionCounterType::SUM}; - bytesAfterCompression_ = {type, - name, - level, - CompressionCounterKey::BYTES_AFTER_COMPRESSION, - CompressionCounterType::SUM}; - bytesBeforeDecompression_ = { - type, - name, - level, - CompressionCounterKey::BYTES_BEFORE_DECOMPRESSION, - CompressionCounterType::SUM}; - bytesAfterDecompression_ = { - type, - name, - level, - CompressionCounterKey::BYTES_AFTER_DECOMPRESSION, - CompressionCounterType::SUM}; - compressions_ = {type, - name, - level, - CompressionCounterKey::COMPRESSIONS, - CompressionCounterType::SUM}; - decompressions_ = {type, - name, - level, - CompressionCounterKey::DECOMPRESSIONS, - CompressionCounterType::SUM}; - compressionMilliseconds_ = {type, - name, - level, - CompressionCounterKey::COMPRESSION_MILLISECONDS, - CompressionCounterType::SUM}; - decompressionMilliseconds_ = { - type, - name, - level, - CompressionCounterKey::DECOMPRESSION_MILLISECONDS, - CompressionCounterType::SUM}; - } -} - -namespace { -constexpr uint32_t kLoggingRate = 50; - -class Timer { - public: - explicit Timer(folly::detail::CompressionCounter& counter) - : counter_(&counter) {} - - ~Timer() { - *counter_ += timer_.elapsed().count(); - } - - private: - folly::detail::CompressionCounter* counter_; - stop_watch timer_; -}; -} // namespace +Codec::Codec(CodecType type) : type_(type) { } // Ensure consistent behavior in the nullptr case std::unique_ptr Codec::compress(const IOBuf* data) { if (data == nullptr) { throw std::invalid_argument("Codec: data must not be nullptr"); } - const uint64_t len = data->computeChainDataLength(); + uint64_t len = data->computeChainDataLength(); if (len > maxUncompressedLength()) { throw std::runtime_error("Codec: uncompressed length too large"); } - bool const logging = folly::Random::oneIn(kLoggingRate); - folly::Optional const timer = - logging ? Timer(compressionMilliseconds_) : folly::Optional(); - auto result = doCompress(data); - if (logging) { - compressions_++; - bytesBeforeCompression_ += len; - bytesAfterCompression_ += result->computeChainDataLength(); - } - return result; + + return doCompress(data); } std::string Codec::compress(const StringPiece data) { @@ -164,16 +85,8 @@ std::string Codec::compress(const StringPiece data) { if (len > maxUncompressedLength()) { throw std::runtime_error("Codec: uncompressed length too large"); } - bool const logging = folly::Random::oneIn(kLoggingRate); - folly::Optional const timer = - logging ? Timer(compressionMilliseconds_) : folly::Optional(); - auto result = doCompressString(data); - if (logging) { - compressions_++; - bytesBeforeCompression_ += len; - bytesAfterCompression_ += result.size(); - } - return result; + + return doCompressString(data); } std::unique_ptr Codec::uncompress( @@ -197,16 +110,7 @@ std::unique_ptr Codec::uncompress( return IOBuf::create(0); } - bool const logging = folly::Random::oneIn(kLoggingRate); - folly::Optional const timer = - logging ? Timer(decompressionMilliseconds_) : folly::Optional(); - auto result = doUncompress(data, uncompressedLength); - if (logging) { - decompressions_++; - bytesBeforeDecompression_ += data->computeChainDataLength(); - bytesAfterDecompression_ += result->computeChainDataLength(); - } - return result; + return doUncompress(data, uncompressedLength); } std::string Codec::uncompress( @@ -227,16 +131,7 @@ std::string Codec::uncompress( return ""; } - bool const logging = folly::Random::oneIn(kLoggingRate); - folly::Optional const timer = - logging ? Timer(decompressionMilliseconds_) : folly::Optional(); - auto result = doUncompressString(data, uncompressedLength); - if (logging) { - decompressions_++; - bytesBeforeDecompression_ += data.size(); - bytesAfterDecompression_ += result.size(); - } - return result; + return doUncompressString(data, uncompressedLength); } bool Codec::needsUncompressedLength() const { @@ -656,24 +551,23 @@ std::unique_ptr LZ4Codec::create(int level, CodecType type) { return std::make_unique(level, type); } -static bool lz4ConvertLevel(int level) { +LZ4Codec::LZ4Codec(int level, CodecType type) : Codec(type) { + DCHECK(type == CodecType::LZ4 || type == CodecType::LZ4_VARINT_SIZE); + switch (level) { - case 1: case COMPRESSION_LEVEL_FASTEST: case COMPRESSION_LEVEL_DEFAULT: - return 1; - case 2: + level = 1; + break; case COMPRESSION_LEVEL_BEST: - return 2; + level = 2; + break; } - throw std::invalid_argument( - to("LZ4Codec: invalid level: ", level)); -} - -LZ4Codec::LZ4Codec(int level, CodecType type) - : Codec(type, lz4ConvertLevel(level)), - highCompression_(lz4ConvertLevel(level) > 1) { - DCHECK(type == CodecType::LZ4 || type == CodecType::LZ4_VARINT_SIZE); + if (level < 1 || level > 2) { + throw std::invalid_argument(to( + "LZ4Codec: invalid level: ", level)); + } + highCompression_ = (level > 1); } bool LZ4Codec::doNeedsUncompressedLength() const { @@ -845,20 +739,20 @@ void LZ4FrameCodec::resetDCtx() { dirty_ = false; } -static int lz4fConvertLevel(int level) { +LZ4FrameCodec::LZ4FrameCodec(int level, CodecType type) : Codec(type) { + DCHECK(type == CodecType::LZ4_FRAME); switch (level) { case COMPRESSION_LEVEL_FASTEST: case COMPRESSION_LEVEL_DEFAULT: - return 0; + level_ = 0; + break; case COMPRESSION_LEVEL_BEST: - return 16; + level_ = 16; + break; + default: + level_ = level; + break; } - return level; -} - -LZ4FrameCodec::LZ4FrameCodec(int level, CodecType type) - : Codec(type, lz4fConvertLevel(level)), level_(lz4fConvertLevel(level)) { - DCHECK(type == CodecType::LZ4_FRAME); } LZ4FrameCodec::~LZ4FrameCodec() { @@ -1499,26 +1393,25 @@ std::unique_ptr ZSTDStreamCodec::createStream( return make_unique(level, type); } -static int zstdConvertLevel(int level) { +ZSTDStreamCodec::ZSTDStreamCodec(int level, CodecType type) + : StreamCodec(type) { + DCHECK(type == CodecType::ZSTD); switch (level) { case COMPRESSION_LEVEL_FASTEST: - return 1; + level = 1; + break; case COMPRESSION_LEVEL_DEFAULT: - return 1; + level = 1; + break; case COMPRESSION_LEVEL_BEST: - return 19; + level = 19; + break; } if (level < 1 || level > ZSTD_maxCLevel()) { throw std::invalid_argument( to("ZSTD: invalid level: ", level)); } - return level; -} - -ZSTDStreamCodec::ZSTDStreamCodec(int level, CodecType type) - : StreamCodec(type, zstdConvertLevel(level)), - level_(zstdConvertLevel(level)) { - DCHECK(type == CodecType::ZSTD); + level_ = level; } bool ZSTDStreamCodec::doNeedsUncompressedLength() const { @@ -2017,7 +1910,7 @@ void AutomaticCodec::addCodecIfSupported(CodecType type) { AutomaticCodec::AutomaticCodec( std::vector> customCodecs, std::unique_ptr terminalCodec) - : Codec(CodecType::USER_DEFINED, folly::none, "auto"), + : Codec(CodecType::USER_DEFINED), codecs_(std::move(customCodecs)), terminalCodec_(std::move(terminalCodec)) { // Fastest -> slowest diff --git a/folly/compression/Compression.h b/folly/compression/Compression.h index 53f47181..456afcf2 100644 --- a/folly/compression/Compression.h +++ b/folly/compression/Compression.h @@ -24,7 +24,6 @@ #include #include -#include #include /** @@ -186,11 +185,7 @@ class Codec { folly::Optional uncompressedLength = folly::none) const; protected: - Codec( - CodecType type, - folly::Optional level = folly::none, - folly::StringPiece name = {}, - bool counters = true); + explicit Codec(CodecType type); public: /** @@ -236,14 +231,6 @@ class Codec { folly::Optional uncompressedLength) const; CodecType type_; - folly::detail::CompressionCounter bytesBeforeCompression_; - folly::detail::CompressionCounter bytesAfterCompression_; - folly::detail::CompressionCounter bytesBeforeDecompression_; - folly::detail::CompressionCounter bytesAfterDecompression_; - folly::detail::CompressionCounter compressions_; - folly::detail::CompressionCounter decompressions_; - folly::detail::CompressionCounter compressionMilliseconds_; - folly::detail::CompressionCounter decompressionMilliseconds_; }; class StreamCodec : public Codec { @@ -364,12 +351,7 @@ class StreamCodec : public Codec { FlushOp flushOp = StreamCodec::FlushOp::NONE); protected: - StreamCodec( - CodecType type, - folly::Optional level = folly::none, - folly::StringPiece name = {}, - bool counters = true) - : Codec(type, std::move(level), name, counters) {} + explicit StreamCodec(CodecType type) : Codec(type) {} // Returns the uncompressed length last passed to resetStream() or none if it // hasn't been called yet. diff --git a/folly/compression/Counters.h b/folly/compression/Counters.h index b46d2e59..26c6740d 100644 --- a/folly/compression/Counters.h +++ b/folly/compression/Counters.h @@ -32,10 +32,6 @@ enum class CompressionCounterKey { BYTES_AFTER_COMPRESSION = 1, BYTES_BEFORE_DECOMPRESSION = 2, BYTES_AFTER_DECOMPRESSION = 3, - COMPRESSIONS = 4, - DECOMPRESSIONS = 5, - COMPRESSION_MILLISECONDS = 6, - DECOMPRESSION_MILLISECONDS = 7, }; enum class CompressionCounterType { diff --git a/folly/compression/Zlib.cpp b/folly/compression/Zlib.cpp index 55034647..8547a816 100644 --- a/folly/compression/Zlib.cpp +++ b/folly/compression/Zlib.cpp @@ -194,32 +194,28 @@ std::unique_ptr ZlibStreamCodec::createStream( return std::make_unique(options, level); } -static bool inBounds(int value, int low, int high) { - return (value >= low) && (value <= high); -} - -static int zlibConvertLevel(int level) { +ZlibStreamCodec::ZlibStreamCodec(Options options, int level) + : StreamCodec(getCodecType(options)) { switch (level) { case COMPRESSION_LEVEL_FASTEST: - return 1; + level = 1; + break; case COMPRESSION_LEVEL_DEFAULT: - return 6; + level = Z_DEFAULT_COMPRESSION; + break; case COMPRESSION_LEVEL_BEST: - return 9; + level = 9; + break; } - if (!inBounds(level, 0, 9)) { + auto inBounds = [](int value, int low, int high) { + return (value >= low) && (value <= high); + }; + + if (level != Z_DEFAULT_COMPRESSION && !inBounds(level, 0, 9)) { throw std::invalid_argument( to("ZlibStreamCodec: invalid level: ", level)); } - return level; -} - -ZlibStreamCodec::ZlibStreamCodec(Options options, int level) - : StreamCodec( - getCodecType(options), - zlibConvertLevel(level), - getCodecType(options) == CodecType::GZIP ? "gzip" : "zlib"), - level_(zlibConvertLevel(level)) { + level_ = level; options_ = options; // Although zlib allows a windowSize of 8..15, a value of 8 is not diff --git a/folly/compression/test/CompressionTest.cpp b/folly/compression/test/CompressionTest.cpp index a1725765..9483979a 100644 --- a/folly/compression/test/CompressionTest.cpp +++ b/folly/compression/test/CompressionTest.cpp @@ -26,6 +26,7 @@ #include #include +#include #include #include #include @@ -1488,3 +1489,14 @@ INSTANTIATE_TEST_CASE_P( } // namespace test } // namespace io } // namespace folly + +int main(int argc, char *argv[]) { + testing::InitGoogleTest(&argc, argv); + gflags::ParseCommandLineFlags(&argc, &argv, true); + + auto ret = RUN_ALL_TESTS(); + if (!ret) { + folly::runBenchmarksOnFlag(); + } + return ret; +}