Skip to content

quiche: Implement QuicStreamBufferAllocator #6550

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jmarantz merged 6 commits into from
Apr 12, 2019
Merged

quiche: Implement QuicStreamBufferAllocator #6550

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fe99129
Implement QuicStreamBufferAllocator
danzh1989 Apr 10, 2019
e94e659
switch to use SimpleBufferAllocator
danzh1989 Apr 11, 2019
1591f79
Merge branch 'master' into allocator
danzh1989 Apr 11, 2019
827b033
fix left over merge conflict
danzh1989 Apr 11, 2019
1115d12
remove cerr
danzh1989 Apr 11, 2019
01a2e33
uncondonize start_mem
danzh1989 Apr 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions bazel/external/quiche.BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ cc_library(
"quiche/quic/platform/api/quic_ptr_util.h",
"quiche/quic/platform/api/quic_reference_counted.h",
"quiche/quic/platform/api/quic_server_stats.h",
"quiche/quic/platform/api/quic_stream_buffer_allocator.h",
"quiche/quic/platform/api/quic_string_piece.h",
"quiche/quic/platform/api/quic_test_output.h",
"quiche/quic/platform/api/quic_uint128.h",
Expand Down Expand Up @@ -223,6 +224,20 @@ cc_library(
deps = [":quic_platform"],
)

cc_library(
name = "quic_buffer_allocator_lib",
srcs = [
"quiche/quic/core/quic_buffer_allocator.cc",
"quiche/quic/core/quic_simple_buffer_allocator.cc",
],
hdrs = [
"quiche/quic/core/quic_buffer_allocator.h",
"quiche/quic/core/quic_simple_buffer_allocator.h",
],
visibility = ["//visibility:public"],
deps = [":quic_platform_export"],
)

envoy_cc_test(
name = "http2_platform_test",
srcs = envoy_select_quiche(
Expand Down
6 changes: 3 additions & 3 deletions bazel/repository_locations.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,8 +236,8 @@ REPOSITORY_LOCATIONS = dict(
urls = ["https://github.com/google/subpar/archive/1.3.0.tar.gz"],
),
com_googlesource_quiche = dict(
# Static snapshot of https://quiche.googlesource.com/quiche/+archive/4fbea5de9afdf30611b27afd54c45a596944f9c2.tar.gz
sha256 = "2cf9f5ea62a03ca0d8773fe4f56949b72c28ac5b1bcf43d850a571f4e32add2a",
urls = ["https://storage.googleapis.com/quiche-envoy-integration/4fbea5de9afdf30611b27afd54c45a596944f9c2.tar.gz"],
# Static snapshot of https://quiche.googlesource.com/quiche/+archive/ba6354aa1b39f3d9788ead909ad3e678ac863938.tar.gz
sha256 = "4598537810c3d343c32333c5367fcb652638018118f7f4e844e080405d9e73bb",
urls = ["https://storage.googleapis.com/quiche-envoy-integration/ba6354aa1b39f3d9788ead909ad3e678ac863938.tar.gz"],
),
)
6 changes: 5 additions & 1 deletion source/extensions/quic_listeners/quiche/platform/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ envoy_cc_library(
"quic_ptr_util_impl.h",
"quic_server_stats_impl.h",
"quic_string_piece_impl.h",
"quic_stream_buffer_allocator_impl.h",
"quic_uint128_impl.h",
] + envoy_select_quiche([
"quic_expect_bug_impl.h",
Expand All @@ -121,7 +122,10 @@ envoy_cc_library(
"googletest",
],
visibility = ["//visibility:public"],
deps = ["@com_googlesource_quiche//:quic_platform_export"] + envoy_select_quiche([
deps = [
"@com_googlesource_quiche//:quic_platform_export",
"@com_googlesource_quiche//:quic_buffer_allocator_lib",
] + envoy_select_quiche([
":quic_platform_logging_impl_lib",
"//include/envoy/thread:thread_interface",
"//source/common/common:assert_lib",
Expand Down
18 changes: 18 additions & 0 deletions source/extensions/quic_listeners/quiche/platform/quic_stream_buffer_allocator_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#pragma once

// NOLINT(namespace-envoy)

// This file is part of the QUICHE platform implementation, and is not to be
// consumed or referenced directly by other Envoy code. It serves purely as a
// porting layer for QUICHE.

#include "quiche/quic/core/quic_simple_buffer_allocator.h"

namespace quic {

// Implements the interface required by
// https://quiche.googlesource.com/quiche/+/refs/heads/master/quic/platform/api/quic_stream_buffer_allocator.h
// with the default implementation provided by QUICHE.
using QuicStreamBufferAllocatorImpl = SimpleBufferAllocator;

} // namespace quic
2 changes: 2 additions & 0 deletions test/extensions/quic_listeners/quiche/platform/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ envoy_cc_test(
],
external_deps = ["quiche_quic_platform"],
deps = [
"//source/common/memory:stats_lib",
"//test/common/stats:stat_test_utility_lib",
"//test/extensions/transport_sockets/tls:ssl_test_utils",
"//test/mocks/api:api_mocks",
"//test/test_common:logging_lib",
Expand Down
24 changes: 21 additions & 3 deletions test/extensions/quic_listeners/quiche/platform/quic_platform_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@
#include <fstream>
#include <unordered_set>

#include "common/memory/stats.h"
#include "common/network/utility.h"

#include "test/common/stats/stat_test_utility.h"
#include "test/extensions/transport_sockets/tls/ssl_test_utility.h"
#include "test/mocks/api/mocks.h"
#include "test/test_common/environment.h"
Expand Down Expand Up @@ -42,20 +44,20 @@
#include "quiche/quic/platform/api/quic_server_stats.h"
#include "quiche/quic/platform/api/quic_sleep.h"
#include "quiche/quic/platform/api/quic_stack_trace.h"
#include "quiche/quic/platform/api/quic_stream_buffer_allocator.h"
#include "quiche/quic/platform/api/quic_string_piece.h"
#include "quiche/quic/platform/api/quic_test_output.h"
#include "quiche/quic/platform/api/quic_thread.h"
#include "quiche/quic/platform/api/quic_uint128.h"

using testing::_;
using testing::HasSubstr;

// Basic tests to validate functioning of the QUICHE quic platform
// implementation. For platform APIs in which the implementation is a simple
// typedef/passthrough to a std:: or absl:: construct, the tests are kept
// minimal, and serve primarily to verify the APIs compile and link without
// issue.

using testing::_;
using testing::HasSubstr;
using testing::Return;

namespace quic {
Expand Down Expand Up @@ -572,5 +574,21 @@ TEST_F(QuicPlatformTest, FailToPickUnsedPort) {
EXPECT_DEATH_LOG_TO_STDERR(QuicPickUnusedPortOrDie(), "Failed to pick a port for test.");
}

TEST_F(QuicPlatformTest, TestEnvoyQuicBufferAllocator) {
bool deterministic_stats = Envoy::Stats::TestUtil::hasDeterministicMallocStats();
const size_t start_mem = Envoy::Memory::Stats::totalCurrentlyAllocated();
QuicStreamBufferAllocator allocator;
char* p = allocator.New(1024);
if (deterministic_stats) {
EXPECT_LT(start_mem, Envoy::Memory::Stats::totalCurrentlyAllocated());
}
EXPECT_NE(nullptr, p);
memset(p, 'a', 1024);
allocator.Delete(p);
Copy link
Contributor

@jmarantz jmarantz Apr 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could also in this test ensure that the total allocated bytes was the same before and after this delete, via Memory::Stats::totalCurrentlyAllocated() before and after. See test/common/stats/thread_local_store_test.cc , test HeapStatsThreadLocalStoreTest.MemoryWithoutTls for an example. Only do that test if TestUtil::hasDeterministicMallocStats() is true.

Copy link
Contributor Author

@danzh2010 danzh2010 Apr 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

if (deterministic_stats) {
EXPECT_EQ(start_mem, Envoy::Memory::Stats::totalCurrentlyAllocated());
}
}

} // namespace
} // namespace quic