Commit e9134b5e authored by Bence Béky's avatar Bence Béky Committed by Commit Bot

Remove methods only used in tests.

Remove HpackStringDecoder:StartOnly(), StartAndDecoderLength(), and
StartSpecialCaseShort().

This CL lands internal change 170871163 by bnc.

BUG=488484

Change-Id: I5ea3d3650980816d3bd0729a277d5786b5e5e974
Reviewed-on: https://chromium-review.googlesource.com/688714Reviewed-by: default avatarHelen Li <xunjieli@chromium.org>
Commit-Queue: Bence Béky <bnc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506802}
parent 470dd1cc
...@@ -32,11 +32,6 @@ namespace net { ...@@ -32,11 +32,6 @@ namespace net {
// Resume() when more input is available, repeating until kDecodeInProgress is // Resume() when more input is available, repeating until kDecodeInProgress is
// not returned. If kDecodeDone or kDecodeError is returned, then Resume() must // not returned. If kDecodeDone or kDecodeError is returned, then Resume() must
// not be called until Start() has been called to start decoding a new string. // not be called until Start() has been called to start decoding a new string.
//
// There are 3 variants of Start in this class, participants in a performance
// experiment. Perflab experiments show it is generally fastest to call
// StartSpecialCaseShort rather than StartOnly (~9% slower) or
// StartAndDecodeLength (~10% slower).
class HTTP2_EXPORT_PRIVATE HpackStringDecoder { class HTTP2_EXPORT_PRIVATE HpackStringDecoder {
public: public:
enum StringDecoderState { enum StringDecoderState {
...@@ -45,31 +40,8 @@ class HTTP2_EXPORT_PRIVATE HpackStringDecoder { ...@@ -45,31 +40,8 @@ class HTTP2_EXPORT_PRIVATE HpackStringDecoder {
kResumeDecodingLength, kResumeDecodingLength,
}; };
// TODO(jamessynge): Get rid of all but one of the Start and Resume methods
// after all of the HPACK decoder is checked in and has been perf tested.
template <class Listener> template <class Listener>
DecodeStatus Start(DecodeBuffer* db, Listener* cb) { DecodeStatus Start(DecodeBuffer* db, Listener* cb) {
return StartSpecialCaseShort(db, cb);
}
template <class Listener>
DecodeStatus StartOnly(DecodeBuffer* db, Listener* cb) {
state_ = kStartDecodingLength;
return Resume(db, cb);
}
template <class Listener>
DecodeStatus StartAndDecodeLength(DecodeBuffer* db, Listener* cb) {
DecodeStatus status;
if (StartDecodingLength(db, cb, &status)) {
state_ = kDecodingString;
return DecodeString(db, cb);
}
return status;
}
template <class Listener>
DecodeStatus StartSpecialCaseShort(DecodeBuffer* db, Listener* cb) {
// Fast decode path is used if the string is under 127 bytes and the // Fast decode path is used if the string is under 127 bytes and the
// entire length of the string is in the decode buffer. More than 83% of // entire length of the string is in the decode buffer. More than 83% of
// string lengths are encoded in just one byte. // string lengths are encoded in just one byte.
......
...@@ -25,35 +25,14 @@ const bool kMayReturnZeroOnFirst = false; ...@@ -25,35 +25,14 @@ const bool kMayReturnZeroOnFirst = false;
const bool kCompressed = true; const bool kCompressed = true;
const bool kUncompressed = false; const bool kUncompressed = false;
enum StartMethod { class HpackStringDecoderTest : public RandomDecoderTest {
kStart,
kStartOnly,
kStartAndDecodeLength,
kStartSpecialCaseShort,
};
class HpackStringDecoderTest
: public RandomDecoderTest,
public ::testing::WithParamInterface<StartMethod> {
protected: protected:
HpackStringDecoderTest() HpackStringDecoderTest() : listener_(&collector_) {}
: start_method_(GetParam()), listener_(&collector_) {}
DecodeStatus StartDecoding(DecodeBuffer* b) override { DecodeStatus StartDecoding(DecodeBuffer* b) override {
++start_decoding_calls_; ++start_decoding_calls_;
collector_.Clear(); collector_.Clear();
switch (start_method_) {
case kStart:
return decoder_.Start(b, &listener_); return decoder_.Start(b, &listener_);
case kStartOnly:
return decoder_.StartOnly(b, &listener_);
case kStartAndDecodeLength:
return decoder_.StartAndDecodeLength(b, &listener_);
case kStartSpecialCaseShort:
return decoder_.StartSpecialCaseShort(b, &listener_);
default:
return DecodeStatus::kDecodeError;
}
} }
DecodeStatus ResumeDecoding(DecodeBuffer* b) override { DecodeStatus ResumeDecoding(DecodeBuffer* b) override {
...@@ -93,14 +72,13 @@ class HpackStringDecoderTest ...@@ -93,14 +72,13 @@ class HpackStringDecoderTest
}; };
} }
const StartMethod start_method_;
HpackStringDecoder decoder_; HpackStringDecoder decoder_;
HpackStringCollector collector_; HpackStringCollector collector_;
HpackStringDecoderVLoggingListener listener_; HpackStringDecoderVLoggingListener listener_;
size_t start_decoding_calls_ = 0; size_t start_decoding_calls_ = 0;
}; };
TEST_P(HpackStringDecoderTest, DecodeEmptyString) { TEST_F(HpackStringDecoderTest, DecodeEmptyString) {
{ {
Validator validator = ValidateDoneAndEmpty(MakeValidator("", kCompressed)); Validator validator = ValidateDoneAndEmpty(MakeValidator("", kCompressed));
const char kData[] = {0x80u}; const char kData[] = {0x80u};
...@@ -121,7 +99,7 @@ TEST_P(HpackStringDecoderTest, DecodeEmptyString) { ...@@ -121,7 +99,7 @@ TEST_P(HpackStringDecoderTest, DecodeEmptyString) {
} }
} }
TEST_P(HpackStringDecoderTest, DecodeShortString) { TEST_F(HpackStringDecoderTest, DecodeShortString) {
{ {
// Make sure it stops after decoding the non-empty string. // Make sure it stops after decoding the non-empty string.
Validator validator = Validator validator =
...@@ -141,7 +119,7 @@ TEST_P(HpackStringDecoderTest, DecodeShortString) { ...@@ -141,7 +119,7 @@ TEST_P(HpackStringDecoderTest, DecodeShortString) {
} }
} }
TEST_P(HpackStringDecoderTest, DecodeLongStrings) { TEST_F(HpackStringDecoderTest, DecodeLongStrings) {
Http2String name = Random().RandString(1024); Http2String name = Random().RandString(1024);
Http2String value = Random().RandString(65536); Http2String value = Random().RandString(65536);
HpackBlockBuilder hbb; HpackBlockBuilder hbb;
...@@ -173,13 +151,6 @@ TEST_P(HpackStringDecoderTest, DecodeLongStrings) { ...@@ -173,13 +151,6 @@ TEST_P(HpackStringDecoderTest, DecodeLongStrings) {
EXPECT_EQ(0u, b.Remaining()); EXPECT_EQ(0u, b.Remaining());
} }
INSTANTIATE_TEST_CASE_P(AllStartMethods,
HpackStringDecoderTest,
::testing::Values(kStart,
kStartOnly,
kStartAndDecodeLength,
kStartSpecialCaseShort));
} // namespace } // namespace
} // namespace test } // namespace test
} // namespace net } // namespace net
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment