From 85f9e80d10460f2d8a312d822b7c243bc9aba6c9 Mon Sep 17 00:00:00 2001 From: dutor Date: Wed, 9 Aug 2017 09:15:05 +0800 Subject: [PATCH 01/39] Fix init_max_accept_queue_size --- src/core/lib/iomgr/tcp_server_utils_posix_common.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/lib/iomgr/tcp_server_utils_posix_common.cc b/src/core/lib/iomgr/tcp_server_utils_posix_common.cc index a828bee074a..70c83b83557 100644 --- a/src/core/lib/iomgr/tcp_server_utils_posix_common.cc +++ b/src/core/lib/iomgr/tcp_server_utils_posix_common.cc @@ -55,7 +55,7 @@ static void init_max_accept_queue_size(void) { if (fgets(buf, sizeof buf, fp)) { char *end; long i = strtol(buf, &end, 10); - if (i > 0 && i <= INT_MAX && end && *end == 0) { + if (i > 0 && i <= INT_MAX && end && *end == '\n') { n = (int)i; } } From 572dd8e8970ec660fb281dfde0cf22c0ee20f31c Mon Sep 17 00:00:00 2001 From: Dan Wittmer Date: Mon, 6 Nov 2017 10:50:26 -0800 Subject: [PATCH 02/39] Change adds a version of grpc::testing::interop::RunServer that allows clients to pass in optional condition_variable which will be notified when the server has started and port to use, avoiding the need for callers to work with command line options. Above is used to support clients running the server in a separate thread in the same process as the tests are run to support local only tests. Existing grpc::testing::interop::RunServer calls into new version passing in FLAGS_port and null condition variable. --- test/cpp/interop/interop_server.cc | 15 +++++++++++++-- test/cpp/interop/server_helper.h | 15 +++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/test/cpp/interop/interop_server.cc b/test/cpp/interop/interop_server.cc index 4149724b1e0..bcef9c38b7a 100644 --- a/test/cpp/interop/interop_server.cc +++ b/test/cpp/interop/interop_server.cc @@ -317,9 +317,16 @@ class TestServiceImpl : public TestService::Service { void grpc::testing::interop::RunServer( std::shared_ptr creds) { - GPR_ASSERT(FLAGS_port != 0); + RunServer(creds, FLAGS_port, nullptr); +} + +void grpc::testing::interop::RunServer( + std::shared_ptr creds, + const int port, + std::condition_variable *server_started_condition) { + GPR_ASSERT(port != 0); std::ostringstream server_address; - server_address << "0.0.0.0:" << FLAGS_port; + server_address << "0.0.0.0:" << port; TestServiceImpl service; SimpleRequest request; @@ -333,6 +340,10 @@ void grpc::testing::interop::RunServer( } std::unique_ptr server(builder.BuildAndStart()); gpr_log(GPR_INFO, "Server listening on %s", server_address.str().c_str()); + + // Signal that the server has started. + if (server_started_condition) server_started_condition->notify_all(); + while (!gpr_atm_no_barrier_load(&g_got_sigint)) { gpr_sleep_until(gpr_time_add(gpr_now(GPR_CLOCK_REALTIME), gpr_time_from_seconds(5, GPR_TIMESPAN))); diff --git a/test/cpp/interop/server_helper.h b/test/cpp/interop/server_helper.h index 6af003fe93f..8e7a6dddf77 100644 --- a/test/cpp/interop/server_helper.h +++ b/test/cpp/interop/server_helper.h @@ -19,6 +19,7 @@ #ifndef GRPC_TEST_CPP_INTEROP_SERVER_HELPER_H #define GRPC_TEST_CPP_INTEROP_SERVER_HELPER_H +#include #include #include @@ -50,8 +51,22 @@ class InteropServerContextInspector { namespace interop { extern gpr_atm g_got_sigint; + +/// Run gRPC interop server using port FLAGS_port. +/// +/// \param creds The credentials associated with the server. void RunServer(std::shared_ptr creds); +/// Run gRPC interop server. +/// +/// \param creds The credentials associated with the server. +/// \param port Port to use for the server. +/// \param server_started_condition (optional) Condition variable to used to +/// notify when the server has started. +void RunServer(std::shared_ptr creds, + int port, + std::condition_variable *server_started_condition); + } // namespace interop } // namespace testing } // namespace grpc From 728f1d2c44b112800d3d420a30033d1e3e291b92 Mon Sep 17 00:00:00 2001 From: Dan Wittmer Date: Thu, 16 Nov 2017 14:37:49 -0800 Subject: [PATCH 03/39] s//used --- test/cpp/interop/server_helper.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/cpp/interop/server_helper.h b/test/cpp/interop/server_helper.h index 8e7a6dddf77..bdbea8f653a 100644 --- a/test/cpp/interop/server_helper.h +++ b/test/cpp/interop/server_helper.h @@ -61,8 +61,8 @@ void RunServer(std::shared_ptr creds); /// /// \param creds The credentials associated with the server. /// \param port Port to use for the server. -/// \param server_started_condition (optional) Condition variable to used to -/// notify when the server has started. +/// \param server_started_condition (optional) Condition variable used to notify +/// when the server has started. void RunServer(std::shared_ptr creds, int port, std::condition_variable *server_started_condition); From 6d18fcd3ab5815af5044d416f20f7b684d950e17 Mon Sep 17 00:00:00 2001 From: Dan Wittmer Date: Thu, 16 Nov 2017 16:45:23 -0800 Subject: [PATCH 04/39] Add ServerStartedCondition to hold the mutex, condition variable and condition. Changes allow callers to correctly handle spurious wakeups. --- test/cpp/interop/interop_server.cc | 8 ++++++-- test/cpp/interop/server_helper.h | 12 +++++++++--- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/test/cpp/interop/interop_server.cc b/test/cpp/interop/interop_server.cc index bcef9c38b7a..5b9e229804b 100644 --- a/test/cpp/interop/interop_server.cc +++ b/test/cpp/interop/interop_server.cc @@ -323,7 +323,7 @@ void grpc::testing::interop::RunServer( void grpc::testing::interop::RunServer( std::shared_ptr creds, const int port, - std::condition_variable *server_started_condition) { + ServerStartedCondition *server_started_condition) { GPR_ASSERT(port != 0); std::ostringstream server_address; server_address << "0.0.0.0:" << port; @@ -342,7 +342,11 @@ void grpc::testing::interop::RunServer( gpr_log(GPR_INFO, "Server listening on %s", server_address.str().c_str()); // Signal that the server has started. - if (server_started_condition) server_started_condition->notify_all(); + if (server_started_condition) { + std::unique_lock lock(server_started_condition->mutex); + server_started_condition->server_started = true; + server_started_condition->condition.notify_all(); + } while (!gpr_atm_no_barrier_load(&g_got_sigint)) { gpr_sleep_until(gpr_time_add(gpr_now(GPR_CLOCK_REALTIME), diff --git a/test/cpp/interop/server_helper.h b/test/cpp/interop/server_helper.h index bdbea8f653a..ab5de07c0ea 100644 --- a/test/cpp/interop/server_helper.h +++ b/test/cpp/interop/server_helper.h @@ -52,6 +52,12 @@ namespace interop { extern gpr_atm g_got_sigint; +struct ServerStartedCondition { + std::mutex mutex; + std::condition_variable condition; + bool server_started = false; +}; + /// Run gRPC interop server using port FLAGS_port. /// /// \param creds The credentials associated with the server. @@ -61,11 +67,11 @@ void RunServer(std::shared_ptr creds); /// /// \param creds The credentials associated with the server. /// \param port Port to use for the server. -/// \param server_started_condition (optional) Condition variable used to notify -/// when the server has started. +/// \param server_started_condition (optional) Struct holding mutex, condition +/// variable, and condition used to notify when the server has started. void RunServer(std::shared_ptr creds, int port, - std::condition_variable *server_started_condition); + ServerStartedCondition *server_started_condition); } // namespace interop } // namespace testing From b88ab92af28bea0badf96b320538011abaffa2c6 Mon Sep 17 00:00:00 2001 From: Dan Wittmer Date: Fri, 17 Nov 2017 15:23:32 -0800 Subject: [PATCH 05/39] Fix formatting of RunServer params. --- test/cpp/interop/interop_server.cc | 3 +-- test/cpp/interop/server_helper.h | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/test/cpp/interop/interop_server.cc b/test/cpp/interop/interop_server.cc index 5b9e229804b..f697dda444e 100644 --- a/test/cpp/interop/interop_server.cc +++ b/test/cpp/interop/interop_server.cc @@ -321,8 +321,7 @@ void grpc::testing::interop::RunServer( } void grpc::testing::interop::RunServer( - std::shared_ptr creds, - const int port, + std::shared_ptr creds, const int port, ServerStartedCondition *server_started_condition) { GPR_ASSERT(port != 0); std::ostringstream server_address; diff --git a/test/cpp/interop/server_helper.h b/test/cpp/interop/server_helper.h index ab5de07c0ea..92920b39367 100644 --- a/test/cpp/interop/server_helper.h +++ b/test/cpp/interop/server_helper.h @@ -69,8 +69,7 @@ void RunServer(std::shared_ptr creds); /// \param port Port to use for the server. /// \param server_started_condition (optional) Struct holding mutex, condition /// variable, and condition used to notify when the server has started. -void RunServer(std::shared_ptr creds, - int port, +void RunServer(std::shared_ptr creds, int port, ServerStartedCondition *server_started_condition); } // namespace interop From 472d7c92324213f5b7c0debc5f23720007d910b9 Mon Sep 17 00:00:00 2001 From: Dan Wittmer Date: Fri, 17 Nov 2017 15:49:11 -0800 Subject: [PATCH 06/39] =?UTF-8?q?Fix=20formatting=20-=20missed=20moving=20?= =?UTF-8?q?=E2=80=98*=E2=80=99=20next=20to=20type.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/cpp/interop/interop_server.cc | 2 +- test/cpp/interop/server_helper.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/cpp/interop/interop_server.cc b/test/cpp/interop/interop_server.cc index f697dda444e..7a028111efa 100644 --- a/test/cpp/interop/interop_server.cc +++ b/test/cpp/interop/interop_server.cc @@ -322,7 +322,7 @@ void grpc::testing::interop::RunServer( void grpc::testing::interop::RunServer( std::shared_ptr creds, const int port, - ServerStartedCondition *server_started_condition) { + ServerStartedCondition* server_started_condition) { GPR_ASSERT(port != 0); std::ostringstream server_address; server_address << "0.0.0.0:" << port; diff --git a/test/cpp/interop/server_helper.h b/test/cpp/interop/server_helper.h index 92920b39367..1bf7db1e147 100644 --- a/test/cpp/interop/server_helper.h +++ b/test/cpp/interop/server_helper.h @@ -70,7 +70,7 @@ void RunServer(std::shared_ptr creds); /// \param server_started_condition (optional) Struct holding mutex, condition /// variable, and condition used to notify when the server has started. void RunServer(std::shared_ptr creds, int port, - ServerStartedCondition *server_started_condition); + ServerStartedCondition* server_started_condition); } // namespace interop } // namespace testing From b75db422bdca7a15a10372d660448cb7e15be5ca Mon Sep 17 00:00:00 2001 From: Matt Kwong Date: Mon, 9 Oct 2017 17:53:05 -0700 Subject: [PATCH 07/39] Add multi-vm performance tests to Kokoro --- .gitignore | 1 + .../linux_kokoro_performance_worker_init.sh | 13 ++++ .../prepare_build_linux_perf_multilang_rc | 40 +++++++++++++ .../linux/grpc_full_performance_master.cfg | 25 ++++++++ .../linux/grpc_full_performance_master.sh | 59 +++++++++++++++++++ tools/run_tests/run_performance_tests.py | 12 +++- 6 files changed, 148 insertions(+), 2 deletions(-) create mode 100644 tools/internal_ci/helper_scripts/prepare_build_linux_perf_multilang_rc create mode 100644 tools/internal_ci/linux/grpc_full_performance_master.cfg create mode 100755 tools/internal_ci/linux/grpc_full_performance_master.sh diff --git a/.gitignore b/.gitignore index 5ccad2e4f2f..151bbbde130 100644 --- a/.gitignore +++ b/.gitignore @@ -56,6 +56,7 @@ Gemfile.lock # Temporary test reports report.xml +*/sponge_log.xml latency_trace.txt latency_trace.*.txt diff --git a/tools/gce/linux_kokoro_performance_worker_init.sh b/tools/gce/linux_kokoro_performance_worker_init.sh index ac3d39383bb..460f639f9f0 100755 --- a/tools/gce/linux_kokoro_performance_worker_init.sh +++ b/tools/gce/linux_kokoro_performance_worker_init.sh @@ -114,6 +114,19 @@ sudo apt-get update sudo apt-get install -y dotnet-dev-1.0.0-preview2.1-003155 sudo apt-get install -y dotnet-dev-1.0.1 +# C# 1.0.4 SDK +curl -O https://download.microsoft.com/download/2/4/A/24A06858-E8AC-469B-8AE6-D0CEC9BA982A/dotnet-ubuntu.16.04-x64.1.0.5.tar.gz +sudo mkdir -p /opt/dotnet +sudo tar zxf dotnet-ubuntu.16.04-x64.1.0.5.tar.gz -C /opt/dotnet +sudo ln -s /opt/dotnet/dotnet /usr/local/bin + +# C# .NET dependencies +wget http://security.ubuntu.com/ubuntu/pool/main/i/icu/libicu52_52.1-8ubuntu0.2_amd64.deb +sudo dpkg -i libicu52_52.1-8ubuntu0.2_amd64.deb +wget http://security.ubuntu.com/ubuntu/pool/main/i/icu/libicu55_55.1-7ubuntu0.3_amd64.deb +sudo dpkg -i libicu55_55.1-7ubuntu0.3_amd64.deb +sudo apt-get update && sudo apt-get install -y libicu55 + # Ruby dependencies gpg --keyserver hkp://keys.gnupg.net --recv-keys 409B6B1796C275462A1703113804BB82D39DC0E3 curl -sSL https://get.rvm.io | bash -s stable --ruby diff --git a/tools/internal_ci/helper_scripts/prepare_build_linux_perf_multilang_rc b/tools/internal_ci/helper_scripts/prepare_build_linux_perf_multilang_rc new file mode 100644 index 00000000000..f1031aedf39 --- /dev/null +++ b/tools/internal_ci/helper_scripts/prepare_build_linux_perf_multilang_rc @@ -0,0 +1,40 @@ +#!/bin/bash +# Copyright 2017 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Source this rc script to prepare the environment for linux perf builds + +# Need to increase open files limit and size for perf test +ulimit -n 32768 +ulimit -c unlimited + +# Download non-core gRPC repos +git clone --recursive https://github.com/grpc/grpc-go ./../grpc-go +git clone --recursive https://github.com/grpc/grpc-java ./../grpc-java +git clone --recursive https://github.com/grpc/grpc-node ./../grpc-node + +sudo pip install tabulate + +# Set up Ruby +export PATH="$HOME/.rbenv/bin:$PATH" +eval "$(rbenv init -)" +gem list bundler +gem install bundler --no-ri --no-rdoc + +# Allow SSH to Kokoro performance workers without explicit key verification +gsutil cp gs://grpc-testing-secrets/grpc_kokoro_performance_ssh_keys/id_rsa ~/.ssh +echo -e 'Host grpc-kokoro-performance*\n\tStrictHostKeyChecking no' >> ~/.ssh/config +chmod 600 ~/.ssh/id_rsa ~/.ssh/config + +git submodule update --init diff --git a/tools/internal_ci/linux/grpc_full_performance_master.cfg b/tools/internal_ci/linux/grpc_full_performance_master.cfg new file mode 100644 index 00000000000..8852130a139 --- /dev/null +++ b/tools/internal_ci/linux/grpc_full_performance_master.cfg @@ -0,0 +1,25 @@ +# Copyright 2017 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Config file for the internal CI (in protobuf text format) + +# Location of the continuous shell script in repository. +build_file: "grpc/tools/internal_ci/linux/grpc_full_performance_master.sh" +timeout_mins: 600 +action { + define_artifacts { + regex: "**/*sponge_log.xml" + regex: "**/perf_reports/**" + } +} diff --git a/tools/internal_ci/linux/grpc_full_performance_master.sh b/tools/internal_ci/linux/grpc_full_performance_master.sh new file mode 100755 index 00000000000..2ba23cbd006 --- /dev/null +++ b/tools/internal_ci/linux/grpc_full_performance_master.sh @@ -0,0 +1,59 @@ +#!/usr/bin/env bash +# Copyright 2017 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +set -ex + +# Enter the gRPC repo root +cd $(dirname $0)/../../.. + +source tools/internal_ci/helper_scripts/prepare_build_linux_perf_multilang_rc + +# run 8core client vs 8core server +tools/run_tests/run_performance_tests.py \ + -l c++ csharp ruby java python go php7 php7_protobuf_c \ + --netperf \ + --category scalable \ + --remote_worker_host grpc-kokoro-performance-server-8core grpc-kokoro-performance-client-8core grpc-kokoro-performance-client2-8core \ + -u kbuilder \ + --bq_result_table performance_test.kokoro_performance_experiment \ + --xml_report reports/8core/sponge_log.xml \ + || EXIT_CODE=1 + +# prevent pushing leftover build files to remote hosts in the next step. +git clean -fdxq --exclude=\!sponge_log.xml + +# scalability with 32cores (and upload to a different BQ table) +tools/run_tests/run_performance_tests.py \ + -l c++ java csharp go \ + --netperf \ + --category scalable \ + --remote_worker_host grpc-kokoro-performance-server-32core grpc-kokoro-performance-client-32core grpc-kokoro-performance-client2-32core \ + -u kbuilder \ + --bq_result_table performance_test.kokoro_performance_experiment_32core \ + --xml_report reports/32core/sponge_log.xml \ + || EXIT_CODE=1 + +# prevent pushing leftover build files to remote hosts in the next step. +git clean -fdxq --exclude=\!sponge_log.xml + +# selected scenarios on Windows +tools/run_tests/run_performance_tests.py \ + -l csharp \ + --category scalable \ + --remote_worker_host grpc-kokoro-performance-windows1 grpc-kokoro-performance-windows2 \ + --bq_result_table performance_test.kokoro_performance_experiment_windows \ + --xml_report reports/windows/sponge_log.xml \ + || EXIT_CODE=1 + +exit $EXIT_CODE diff --git a/tools/run_tests/run_performance_tests.py b/tools/run_tests/run_performance_tests.py index 1bbab9e894f..aa305be4660 100755 --- a/tools/run_tests/run_performance_tests.py +++ b/tools/run_tests/run_performance_tests.py @@ -196,7 +196,7 @@ def archive_repo(languages): def prepare_remote_hosts(hosts, prepare_local=False): """Prepares remote hosts (and maybe prepare localhost as well).""" - prepare_timeout = 5*60 + prepare_timeout = 10*60 prepare_jobs = [] for host in hosts: user_at_host = '%s@%s' % (_REMOTE_HOST_USERNAME, host) @@ -229,6 +229,8 @@ def prepare_remote_hosts(hosts, prepare_local=False): def build_on_remote_hosts(hosts, languages=scenario_config.LANGUAGES.keys(), build_local=False): """Builds performance worker on remote hosts (and maybe also locally).""" build_timeout = 15*60 + # Kokoro VMs (which are local only) do not have caching, so they need more time to build + local_build_timeout = 30*60 build_jobs = [] for host in hosts: user_at_host = '%s@%s' % (_REMOTE_HOST_USERNAME, host) @@ -245,7 +247,7 @@ def build_on_remote_hosts(hosts, languages=scenario_config.LANGUAGES.keys(), bui cmdline=['tools/run_tests/performance/build_performance.sh'] + languages, shortname='local_build', environ = {'CONFIG': 'opt'}, - timeout_seconds=build_timeout)) + timeout_seconds=local_build_timeout)) jobset.message('START', 'Building.', do_newline=True) num_failures, _ = jobset.run( build_jobs, newline_on_success=True, maxjobs=10) @@ -483,9 +485,15 @@ def main(): 'generating flamegraphs (e.g., "--perf_args=stat ...")')) argp.add_argument('-f', '--flame_graph_reports', default='perf_reports', type=str, help='Name of directory to output flame graph profiles to, if any are created.') + argp.add_argument('-u', '--remote_host_username', default='', type=str, + help='Use a username that isn\'t "Jenkins" to SSH into remote workers.') args = argp.parse_args() + global _REMOTE_HOST_USERNAME + if args.remote_host_username: + _REMOTE_HOST_USERNAME = args.remote_host_username + languages = set(scenario_config.LANGUAGES[l] for l in itertools.chain.from_iterable( six.iterkeys(scenario_config.LANGUAGES) if x == 'all' From 3267108e5253f43b22cca1061ba9816c0d345b38 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 30 Nov 2017 09:06:20 +0100 Subject: [PATCH 08/39] C# benchmarks provide userTime and systemTime info --- .../Grpc.IntegrationTesting/ClientRunners.cs | 13 ++- .../Grpc.IntegrationTesting/ServerRunners.cs | 13 ++- .../StressTestClient.cs | 6 +- .../Grpc.IntegrationTesting/TimeStats.cs | 90 +++++++++++++++++++ .../WallClockStopwatch.cs | 63 ------------- 5 files changed, 105 insertions(+), 80 deletions(-) create mode 100644 src/csharp/Grpc.IntegrationTesting/TimeStats.cs delete mode 100644 src/csharp/Grpc.IntegrationTesting/WallClockStopwatch.cs diff --git a/src/csharp/Grpc.IntegrationTesting/ClientRunners.cs b/src/csharp/Grpc.IntegrationTesting/ClientRunners.cs index 48905a27154..9d41d34414a 100644 --- a/src/csharp/Grpc.IntegrationTesting/ClientRunners.cs +++ b/src/csharp/Grpc.IntegrationTesting/ClientRunners.cs @@ -131,7 +131,7 @@ namespace Grpc.IntegrationTesting readonly List runnerTasks; readonly CancellationTokenSource stoppedCts = new CancellationTokenSource(); - readonly WallClockStopwatch wallClockStopwatch = new WallClockStopwatch(); + readonly TimeStats timeStats = new TimeStats(); readonly AtomicCounter statsResetCount = new AtomicCounter(); public ClientRunnerImpl(List channels, ClientType clientType, RpcType rpcType, int outstandingRpcsPerChannel, LoadParams loadParams, PayloadConfig payloadConfig, HistogramParams histogramParams, Func profilerFactory) @@ -165,7 +165,7 @@ namespace Grpc.IntegrationTesting hist.GetSnapshot(histogramData, reset); } - var secondsElapsed = wallClockStopwatch.GetElapsedSnapshot(reset).TotalSeconds; + var timeSnapshot = timeStats.GetSnapshot(reset); if (reset) { @@ -173,15 +173,14 @@ namespace Grpc.IntegrationTesting } GrpcEnvironment.Logger.Info("[ClientRunnerImpl.GetStats] GC collection counts: gen0 {0}, gen1 {1}, gen2 {2}, (histogram reset count:{3}, seconds since reset: {4})", - GC.CollectionCount(0), GC.CollectionCount(1), GC.CollectionCount(2), statsResetCount.Count, secondsElapsed); + GC.CollectionCount(0), GC.CollectionCount(1), GC.CollectionCount(2), statsResetCount.Count, timeSnapshot.WallClockTime.TotalSeconds); - // TODO: populate user time and system time return new ClientStats { Latencies = histogramData, - TimeElapsed = secondsElapsed, - TimeUser = 0, - TimeSystem = 0 + TimeElapsed = timeSnapshot.WallClockTime.TotalSeconds, + TimeUser = timeSnapshot.UserProcessorTime.TotalSeconds, + TimeSystem = timeSnapshot.PrivilegedProcessorTime.TotalSeconds }; } diff --git a/src/csharp/Grpc.IntegrationTesting/ServerRunners.cs b/src/csharp/Grpc.IntegrationTesting/ServerRunners.cs index e1b47744d50..ea29bd74e50 100644 --- a/src/csharp/Grpc.IntegrationTesting/ServerRunners.cs +++ b/src/csharp/Grpc.IntegrationTesting/ServerRunners.cs @@ -117,7 +117,7 @@ namespace Grpc.IntegrationTesting public class ServerRunnerImpl : IServerRunner { readonly Server server; - readonly WallClockStopwatch wallClockStopwatch = new WallClockStopwatch(); + readonly TimeStats timeStats = new TimeStats(); public ServerRunnerImpl(Server server) { @@ -138,17 +138,16 @@ namespace Grpc.IntegrationTesting /// The stats. public ServerStats GetStats(bool reset) { - var secondsElapsed = wallClockStopwatch.GetElapsedSnapshot(reset).TotalSeconds; + var timeSnapshot = timeStats.GetSnapshot(reset); GrpcEnvironment.Logger.Info("[ServerRunner.GetStats] GC collection counts: gen0 {0}, gen1 {1}, gen2 {2}, (seconds since last reset {3})", - GC.CollectionCount(0), GC.CollectionCount(1), GC.CollectionCount(2), secondsElapsed); + GC.CollectionCount(0), GC.CollectionCount(1), GC.CollectionCount(2), timeSnapshot.WallClockTime.TotalSeconds); - // TODO: populate user time and system time return new ServerStats { - TimeElapsed = secondsElapsed, - TimeUser = 0, - TimeSystem = 0 + TimeElapsed = timeSnapshot.WallClockTime.TotalSeconds, + TimeUser = timeSnapshot.UserProcessorTime.TotalSeconds, + TimeSystem = timeSnapshot.PrivilegedProcessorTime.TotalSeconds }; } diff --git a/src/csharp/Grpc.IntegrationTesting/StressTestClient.cs b/src/csharp/Grpc.IntegrationTesting/StressTestClient.cs index 11956e4ac8f..0c623807681 100644 --- a/src/csharp/Grpc.IntegrationTesting/StressTestClient.cs +++ b/src/csharp/Grpc.IntegrationTesting/StressTestClient.cs @@ -243,7 +243,7 @@ namespace Grpc.IntegrationTesting const string GaugeName = "csharp_overall_qps"; readonly Histogram histogram; - readonly WallClockStopwatch wallClockStopwatch = new WallClockStopwatch(); + readonly TimeStats timeStats = new TimeStats(); public MetricsServiceImpl(Histogram histogram) { @@ -280,9 +280,9 @@ namespace Grpc.IntegrationTesting long GetQpsAndReset() { var snapshot = histogram.GetSnapshot(true); - var elapsedSnapshot = wallClockStopwatch.GetElapsedSnapshot(true); + var timeSnapshot = timeStats.GetSnapshot(true); - return (long) (snapshot.Count / elapsedSnapshot.TotalSeconds); + return (long) (snapshot.Count / timeSnapshot.WallClockTime.TotalSeconds); } } } diff --git a/src/csharp/Grpc.IntegrationTesting/TimeStats.cs b/src/csharp/Grpc.IntegrationTesting/TimeStats.cs new file mode 100644 index 00000000000..6aba04c1949 --- /dev/null +++ b/src/csharp/Grpc.IntegrationTesting/TimeStats.cs @@ -0,0 +1,90 @@ +#region Copyright notice and license + +// Copyright 2015 gRPC authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#endregion + +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.IO; +using System.Linq; +using System.Text.RegularExpressions; +using System.Threading; +using System.Threading.Tasks; +using Google.Protobuf; +using Grpc.Core; +using Grpc.Core.Utils; +using NUnit.Framework; +using Grpc.Testing; + +namespace Grpc.IntegrationTesting +{ + /// + /// Snapshottable time statistics. + /// + public class TimeStats + { + readonly object myLock = new object(); + DateTime lastWallClock; + TimeSpan lastUserTime; + TimeSpan lastPrivilegedTime; + + public TimeStats() + { + lastWallClock = DateTime.UtcNow; + lastUserTime = Process.GetCurrentProcess().UserProcessorTime; + lastPrivilegedTime = Process.GetCurrentProcess().PrivilegedProcessorTime; + } + + public Snapshot GetSnapshot(bool reset) + { + lock (myLock) + { + var wallClock = DateTime.UtcNow; + var userTime = Process.GetCurrentProcess().UserProcessorTime; + var privilegedTime = Process.GetCurrentProcess().PrivilegedProcessorTime; + var snapshot = new Snapshot(wallClock - lastWallClock, userTime - lastUserTime, privilegedTime - lastPrivilegedTime); + + if (reset) + { + lastWallClock = wallClock; + lastUserTime = userTime; + lastPrivilegedTime = privilegedTime; + } + return snapshot; + } + } + + public class Snapshot + { + public TimeSpan WallClockTime { get; } + public TimeSpan UserProcessorTime { get; } + public TimeSpan PrivilegedProcessorTime { get; } + + public Snapshot(TimeSpan wallClockTime, TimeSpan userProcessorTime, TimeSpan privilegedProcessorTime) + { + this.WallClockTime = wallClockTime; + this.UserProcessorTime = userProcessorTime; + this.PrivilegedProcessorTime = privilegedProcessorTime; + } + + public override string ToString() + { + return string.Format("[TimeStats.Snapshot: wallClock {0}, userProcessor {1}, privilegedProcessor {2}]", WallClockTime, UserProcessorTime, PrivilegedProcessorTime); + } + } + } +} diff --git a/src/csharp/Grpc.IntegrationTesting/WallClockStopwatch.cs b/src/csharp/Grpc.IntegrationTesting/WallClockStopwatch.cs deleted file mode 100644 index 38b58f296c0..00000000000 --- a/src/csharp/Grpc.IntegrationTesting/WallClockStopwatch.cs +++ /dev/null @@ -1,63 +0,0 @@ -#region Copyright notice and license - -// Copyright 2015 gRPC authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#endregion - -using System; -using System.Collections.Generic; -using System.Diagnostics; -using System.IO; -using System.Linq; -using System.Text.RegularExpressions; -using System.Threading; -using System.Threading.Tasks; -using Google.Protobuf; -using Grpc.Core; -using Grpc.Core.Utils; -using NUnit.Framework; -using Grpc.Testing; - -namespace Grpc.IntegrationTesting -{ - /// - /// Snapshottable wall clock stopwatch. - /// - public class WallClockStopwatch - { - long startTicks; - - public WallClockStopwatch() - { - this.startTicks = DateTime.UtcNow.Ticks; - } - - public TimeSpan GetElapsedSnapshot(bool reset) - { - var utcNow = DateTime.UtcNow; - - long oldStartTicks; - if (reset) - { - oldStartTicks = Interlocked.Exchange(ref this.startTicks, utcNow.Ticks); - } - else - { - oldStartTicks = this.startTicks; - } - return utcNow - new DateTime(oldStartTicks, DateTimeKind.Utc); - } - } -} From 4f25daa7aff614fe820eb4d7432b0c8658bef81d Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 30 Nov 2017 17:09:30 +0100 Subject: [PATCH 09/39] dont let server shutdown run forever --- test/core/surface/concurrent_connectivity_test.cc | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/core/surface/concurrent_connectivity_test.cc b/test/core/surface/concurrent_connectivity_test.cc index 8fa15ab3313..c7611b0dd14 100644 --- a/test/core/surface/concurrent_connectivity_test.cc +++ b/test/core/surface/concurrent_connectivity_test.cc @@ -49,10 +49,11 @@ #define NUM_OUTER_LOOPS_SHORT_TIMEOUTS 10 #define NUM_INNER_LOOPS_SHORT_TIMEOUTS 100 #define DELAY_MILLIS_SHORT_TIMEOUTS 1 -// in a successful test run, POLL_MILLIS should never be reached beause all runs -// should -// end after the shorter delay_millis +// in a successful test run, POLL_MILLIS should never be reached because all +// runs should end after the shorter delay_millis #define POLL_MILLIS_SHORT_TIMEOUTS 30000 +// it should never take longer that this to shutdown the server +#define SERVER_SHUTDOWN_TIMEOUT 30000 static void* tag(int n) { return (void*)(uintptr_t)n; } static int detag(void* p) { return (int)(uintptr_t)p; } @@ -95,7 +96,8 @@ struct server_thread_args { void server_thread(void* vargs) { struct server_thread_args* args = (struct server_thread_args*)vargs; grpc_event ev; - gpr_timespec deadline = gpr_inf_future(GPR_CLOCK_MONOTONIC); + gpr_timespec deadline = + grpc_timeout_milliseconds_to_deadline(SERVER_SHUTDOWN_TIMEOUT); ev = grpc_completion_queue_next(args->cq, deadline, nullptr); GPR_ASSERT(ev.type == GRPC_OP_COMPLETE); GPR_ASSERT(detag(ev.tag) == 0xd1e); From 36481f5bae42724bd75089a7db2fc682f8ce3f05 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Thu, 30 Nov 2017 11:05:15 -0800 Subject: [PATCH 10/39] Somehow fork.h got omitted from BUILD --- BUILD | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/BUILD b/BUILD index d904a1ec6ee..30b410a2bd1 100644 --- a/BUILD +++ b/BUILD @@ -79,10 +79,11 @@ GRPC_PUBLIC_HDRS = [ "include/grpc/byte_buffer.h", "include/grpc/byte_buffer_reader.h", "include/grpc/compression.h", - "include/grpc/load_reporting.h", + "include/grpc/fork.h", "include/grpc/grpc.h", "include/grpc/grpc_posix.h", "include/grpc/grpc_security_constants.h", + "include/grpc/load_reporting.h", "include/grpc/slice.h", "include/grpc/slice_buffer.h", "include/grpc/status.h", From 360712fe9997b528da0143ebb04d913209a0f8b8 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 18 Oct 2017 10:05:21 -0700 Subject: [PATCH 11/39] Start to get Bazel running different pollers --- bazel/grpc_build_system.bzl | 39 +++++++++++++++++++++++-------- test/core/util/BUILD | 5 ++++ test/core/util/run_with_poller.sh | 19 +++++++++++++++ 3 files changed, 53 insertions(+), 10 deletions(-) create mode 100755 test/core/util/run_with_poller.sh diff --git a/bazel/grpc_build_system.bzl b/bazel/grpc_build_system.bzl index b35ca73745f..a21b049b375 100644 --- a/bazel/grpc_build_system.bzl +++ b/bazel/grpc_build_system.bzl @@ -23,6 +23,9 @@ # each change must be ported from one to the other. # +# The set of pollers to test against if a test exercises polling +POLLERS = ['epollex', 'epollsig', 'epoll1', 'poll', 'poll-cv'] + def grpc_cc_library(name, srcs = [], public_hdrs = [], hdrs = [], external_deps = [], deps = [], standalone = False, language = "C++", testonly = False, visibility = None, @@ -70,19 +73,35 @@ def grpc_proto_library(name, srcs = [], deps = [], well_known_protos = False, generate_mock = generate_mock, ) -def grpc_cc_test(name, srcs = [], deps = [], external_deps = [], args = [], data = [], language = "C++"): +def grpc_cc_test(name, srcs = [], deps = [], external_deps = [], args = [], data = [], uses_polling = True, language = "C++"): copts = [] if language.upper() == "C": copts = ["-std=c99"] - native.cc_test( - name = name, - srcs = srcs, - args = args, - data = data, - deps = deps + ["//external:" + dep for dep in external_deps], - copts = copts, - linkopts = ["-pthread"], - ) + args = { + 'name': name, + 'srcs': srcs, + 'args': args, + 'data': data, + 'deps': deps + ["//external:" + dep for dep in external_deps], + 'copts': copts, + 'linkopts': ["-pthread"], + } + if uses_polling: + native.cc_binary(testonly=True, **args) + for poller in POLLERS: + native.sh_test( + name = name + '@poller=' + poller, + data = [name], + srcs = [ + '//test/core/util:run_with_poller_sh', + ], + args = [ + poller, + '$(location %s)' % name + ], + ) + else: + native.cc_test(**args) def grpc_cc_binary(name, srcs = [], deps = [], external_deps = [], args = [], data = [], language = "C++", testonly = False, linkshared = False, linkopts = []): copts = [] diff --git a/test/core/util/BUILD b/test/core/util/BUILD index 6443553466f..58de5da1489 100644 --- a/test/core/util/BUILD +++ b/test/core/util/BUILD @@ -121,3 +121,8 @@ sh_library( name = "fuzzer_one_entry_runner", srcs = ["fuzzer_one_entry_runner.sh"], ) + +sh_library( + name = "run_with_poller_sh", + srcs = ["run_with_poller.sh"], +) diff --git a/test/core/util/run_with_poller.sh b/test/core/util/run_with_poller.sh new file mode 100755 index 00000000000..05791457a2c --- /dev/null +++ b/test/core/util/run_with_poller.sh @@ -0,0 +1,19 @@ +#!/bin/sh +# Copyright 2017 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +set -ex +export GRPC_POLL_STRATEGY=$1 +shift +$@ From a6bdf450fdedf470dbe0e7bbdd9dc49ffbe37aab Mon Sep 17 00:00:00 2001 From: Matt Kwong Date: Thu, 30 Nov 2017 14:49:26 -0800 Subject: [PATCH 12/39] Properly preserve sponge_log.xml between performance tests --- tools/internal_ci/linux/grpc_full_performance_master.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/internal_ci/linux/grpc_full_performance_master.sh b/tools/internal_ci/linux/grpc_full_performance_master.sh index 2ba23cbd006..18468395f6f 100755 --- a/tools/internal_ci/linux/grpc_full_performance_master.sh +++ b/tools/internal_ci/linux/grpc_full_performance_master.sh @@ -31,7 +31,7 @@ tools/run_tests/run_performance_tests.py \ || EXIT_CODE=1 # prevent pushing leftover build files to remote hosts in the next step. -git clean -fdxq --exclude=\!sponge_log.xml +git clean -fdxq -e reports # scalability with 32cores (and upload to a different BQ table) tools/run_tests/run_performance_tests.py \ @@ -45,7 +45,7 @@ tools/run_tests/run_performance_tests.py \ || EXIT_CODE=1 # prevent pushing leftover build files to remote hosts in the next step. -git clean -fdxq --exclude=\!sponge_log.xml +git clean -fdxq -e reports # selected scenarios on Windows tools/run_tests/run_performance_tests.py \ From 195cf1ebfd5e3ab12d8271e116e1f022a9b23ef6 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Thu, 30 Nov 2017 10:56:06 -0800 Subject: [PATCH 13/39] Move histogram to test/core/util --- BUILD | 5 +- CMakeLists.txt | 63 ++++--- Makefile | 79 ++++----- build.yaml | 22 +-- config.m4 | 1 - config.w32 | 1 - gRPC-Core.podspec | 2 - grpc.def | 15 -- grpc.gemspec | 2 - grpc.gyp | 3 +- include/grpc/module.modulemap | 1 - include/grpc/support/histogram.h | 64 ------- package.xml | 2 - src/python/grpcio/grpc_core_dependencies.py | 1 - src/ruby/ext/grpc/rb_grpc_imports.generated.c | 30 ---- src/ruby/ext/grpc/rb_grpc_imports.generated.h | 46 ----- test/core/fling/client.cc | 18 +- .../network_benchmarks/low_level_ping_pong.cc | 18 +- test/core/support/BUILD | 10 -- test/core/support/histogram_test.cc | 163 ------------------ .../core/surface/public_headers_must_be_c89.c | 16 -- test/core/util/BUILD | 25 ++- .../support => test/core/util}/histogram.cc | 62 +++---- test/core/util/histogram.h | 62 +++++++ test/core/util/histogram_test.cc | 163 ++++++++++++++++++ test/cpp/qps/BUILD | 2 +- test/cpp/qps/histogram.h | 36 ++-- test/cpp/qps/qps_interarrival_test.cc | 10 +- test/cpp/qps/qps_worker.cc | 2 +- tools/doxygen/Doxyfile.c++ | 1 - tools/doxygen/Doxyfile.c++.internal | 1 - tools/doxygen/Doxyfile.core | 1 - tools/doxygen/Doxyfile.core.internal | 2 - .../generated/sources_and_headers.json | 36 ++-- tools/run_tests/generated/tests.json | 48 +++--- 35 files changed, 444 insertions(+), 569 deletions(-) delete mode 100644 include/grpc/support/histogram.h delete mode 100644 test/core/support/histogram_test.cc rename {src/core/lib/support => test/core/util}/histogram.cc (72%) create mode 100644 test/core/util/histogram.h create mode 100644 test/core/util/histogram_test.cc diff --git a/BUILD b/BUILD index d904a1ec6ee..82ac73f0147 100644 --- a/BUILD +++ b/BUILD @@ -54,7 +54,6 @@ GPR_PUBLIC_HDRS = [ "include/grpc/support/avl.h", "include/grpc/support/cmdline.h", "include/grpc/support/cpu.h", - "include/grpc/support/histogram.h", "include/grpc/support/host_port.h", "include/grpc/support/log.h", "include/grpc/support/log_windows.h", @@ -79,10 +78,11 @@ GRPC_PUBLIC_HDRS = [ "include/grpc/byte_buffer.h", "include/grpc/byte_buffer_reader.h", "include/grpc/compression.h", - "include/grpc/load_reporting.h", + "include/grpc/fork.h", "include/grpc/grpc.h", "include/grpc/grpc_posix.h", "include/grpc/grpc_security_constants.h", + "include/grpc/load_reporting.h", "include/grpc/slice.h", "include/grpc/slice_buffer.h", "include/grpc/status.h", @@ -446,7 +446,6 @@ grpc_cc_library( "src/core/lib/support/env_posix.cc", "src/core/lib/support/env_windows.cc", "src/core/lib/support/fork.cc", - "src/core/lib/support/histogram.cc", "src/core/lib/support/host_port.cc", "src/core/lib/support/log.cc", "src/core/lib/support/log_android.cc", diff --git a/CMakeLists.txt b/CMakeLists.txt index 760e4fa5852..ab57f662cf2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -427,7 +427,6 @@ add_dependencies(buildtests_c gpr_avl_test) add_dependencies(buildtests_c gpr_cmdline_test) add_dependencies(buildtests_c gpr_cpu_test) add_dependencies(buildtests_c gpr_env_test) -add_dependencies(buildtests_c gpr_histogram_test) add_dependencies(buildtests_c gpr_host_port_test) add_dependencies(buildtests_c gpr_log_test) add_dependencies(buildtests_c gpr_manual_constructor_test) @@ -465,6 +464,7 @@ endif() if(_gRPC_PLATFORM_LINUX) add_dependencies(buildtests_c handshake_server_with_readahead_handshaker) endif() +add_dependencies(buildtests_c histogram_test) add_dependencies(buildtests_c hpack_parser_test) add_dependencies(buildtests_c hpack_table_test) add_dependencies(buildtests_c http_parser_test) @@ -799,7 +799,6 @@ add_library(gpr src/core/lib/support/env_posix.cc src/core/lib/support/env_windows.cc src/core/lib/support/fork.cc - src/core/lib/support/histogram.cc src/core/lib/support/host_port.cc src/core/lib/support/log.cc src/core/lib/support/log_android.cc @@ -869,7 +868,6 @@ foreach(_hdr include/grpc/support/avl.h include/grpc/support/cmdline.h include/grpc/support/cpu.h - include/grpc/support/histogram.h include/grpc/support/host_port.h include/grpc/support/log.h include/grpc/support/log_windows.h @@ -1617,6 +1615,7 @@ add_library(grpc_test_util test/core/iomgr/endpoint_tests.cc test/core/util/debugger_macros.cc test/core/util/grpc_profiler.cc + test/core/util/histogram.cc test/core/util/memory_counters.cc test/core/util/mock_endpoint.cc test/core/util/parse_hexstring.cc @@ -1885,6 +1884,7 @@ add_library(grpc_test_util_unsecure test/core/iomgr/endpoint_tests.cc test/core/util/debugger_macros.cc test/core/util/grpc_profiler.cc + test/core/util/histogram.cc test/core/util/memory_counters.cc test/core/util/mock_endpoint.cc test/core/util/parse_hexstring.cc @@ -2671,7 +2671,6 @@ foreach(_hdr include/grpc/support/avl.h include/grpc/support/cmdline.h include/grpc/support/cpu.h - include/grpc/support/histogram.h include/grpc/support/host_port.h include/grpc/support/log.h include/grpc/support/log_windows.h @@ -3158,7 +3157,6 @@ foreach(_hdr include/grpc/support/avl.h include/grpc/support/cmdline.h include/grpc/support/cpu.h - include/grpc/support/histogram.h include/grpc/support/host_port.h include/grpc/support/log.h include/grpc/support/log_windows.h @@ -3905,7 +3903,6 @@ foreach(_hdr include/grpc/support/avl.h include/grpc/support/cmdline.h include/grpc/support/cpu.h - include/grpc/support/histogram.h include/grpc/support/host_port.h include/grpc/support/log.h include/grpc/support/log_windows.h @@ -6259,33 +6256,6 @@ target_link_libraries(gpr_env_test endif (gRPC_BUILD_TESTS) if (gRPC_BUILD_TESTS) -add_executable(gpr_histogram_test - test/core/support/histogram_test.cc -) - - -target_include_directories(gpr_histogram_test - PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} - PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include - PRIVATE ${BORINGSSL_ROOT_DIR}/include - PRIVATE ${PROTOBUF_ROOT_DIR}/src - PRIVATE ${BENCHMARK_ROOT_DIR}/include - PRIVATE ${ZLIB_ROOT_DIR} - PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/third_party/zlib - PRIVATE ${CARES_INCLUDE_DIR} - PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/third_party/cares/cares - PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/third_party/gflags/include -) - -target_link_libraries(gpr_histogram_test - ${_gRPC_ALLTARGETS_LIBRARIES} - gpr_test_util - gpr -) - -endif (gRPC_BUILD_TESTS) -if (gRPC_BUILD_TESTS) - add_executable(gpr_host_port_test test/core/support/host_port_test.cc ) @@ -7221,6 +7191,33 @@ endif() endif (gRPC_BUILD_TESTS) if (gRPC_BUILD_TESTS) +add_executable(histogram_test + test/core/util/histogram_test.cc +) + + +target_include_directories(histogram_test + PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} + PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include + PRIVATE ${BORINGSSL_ROOT_DIR}/include + PRIVATE ${PROTOBUF_ROOT_DIR}/src + PRIVATE ${BENCHMARK_ROOT_DIR}/include + PRIVATE ${ZLIB_ROOT_DIR} + PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/third_party/zlib + PRIVATE ${CARES_INCLUDE_DIR} + PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/third_party/cares/cares + PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/third_party/gflags/include +) + +target_link_libraries(histogram_test + ${_gRPC_ALLTARGETS_LIBRARIES} + grpc_test_util + gpr +) + +endif (gRPC_BUILD_TESTS) +if (gRPC_BUILD_TESTS) + add_executable(hpack_parser_test test/core/transport/chttp2/hpack_parser_test.cc ) diff --git a/Makefile b/Makefile index 3ce30059241..bd83244a616 100644 --- a/Makefile +++ b/Makefile @@ -988,7 +988,6 @@ gpr_avl_test: $(BINDIR)/$(CONFIG)/gpr_avl_test gpr_cmdline_test: $(BINDIR)/$(CONFIG)/gpr_cmdline_test gpr_cpu_test: $(BINDIR)/$(CONFIG)/gpr_cpu_test gpr_env_test: $(BINDIR)/$(CONFIG)/gpr_env_test -gpr_histogram_test: $(BINDIR)/$(CONFIG)/gpr_histogram_test gpr_host_port_test: $(BINDIR)/$(CONFIG)/gpr_host_port_test gpr_log_test: $(BINDIR)/$(CONFIG)/gpr_log_test gpr_manual_constructor_test: $(BINDIR)/$(CONFIG)/gpr_manual_constructor_test @@ -1021,6 +1020,7 @@ grpc_verify_jwt: $(BINDIR)/$(CONFIG)/grpc_verify_jwt handshake_client: $(BINDIR)/$(CONFIG)/handshake_client handshake_server: $(BINDIR)/$(CONFIG)/handshake_server handshake_server_with_readahead_handshaker: $(BINDIR)/$(CONFIG)/handshake_server_with_readahead_handshaker +histogram_test: $(BINDIR)/$(CONFIG)/histogram_test hpack_parser_fuzzer_test: $(BINDIR)/$(CONFIG)/hpack_parser_fuzzer_test hpack_parser_test: $(BINDIR)/$(CONFIG)/hpack_parser_test hpack_table_test: $(BINDIR)/$(CONFIG)/hpack_table_test @@ -1383,7 +1383,6 @@ buildtests_c: privatelibs_c \ $(BINDIR)/$(CONFIG)/gpr_cmdline_test \ $(BINDIR)/$(CONFIG)/gpr_cpu_test \ $(BINDIR)/$(CONFIG)/gpr_env_test \ - $(BINDIR)/$(CONFIG)/gpr_histogram_test \ $(BINDIR)/$(CONFIG)/gpr_host_port_test \ $(BINDIR)/$(CONFIG)/gpr_log_test \ $(BINDIR)/$(CONFIG)/gpr_manual_constructor_test \ @@ -1413,6 +1412,7 @@ buildtests_c: privatelibs_c \ $(BINDIR)/$(CONFIG)/handshake_client \ $(BINDIR)/$(CONFIG)/handshake_server \ $(BINDIR)/$(CONFIG)/handshake_server_with_readahead_handshaker \ + $(BINDIR)/$(CONFIG)/histogram_test \ $(BINDIR)/$(CONFIG)/hpack_parser_test \ $(BINDIR)/$(CONFIG)/hpack_table_test \ $(BINDIR)/$(CONFIG)/http_parser_test \ @@ -1829,8 +1829,6 @@ test_c: buildtests_c $(Q) $(BINDIR)/$(CONFIG)/gpr_cpu_test || ( echo test gpr_cpu_test failed ; exit 1 ) $(E) "[RUN] Testing gpr_env_test" $(Q) $(BINDIR)/$(CONFIG)/gpr_env_test || ( echo test gpr_env_test failed ; exit 1 ) - $(E) "[RUN] Testing gpr_histogram_test" - $(Q) $(BINDIR)/$(CONFIG)/gpr_histogram_test || ( echo test gpr_histogram_test failed ; exit 1 ) $(E) "[RUN] Testing gpr_host_port_test" $(Q) $(BINDIR)/$(CONFIG)/gpr_host_port_test || ( echo test gpr_host_port_test failed ; exit 1 ) $(E) "[RUN] Testing gpr_log_test" @@ -1887,6 +1885,8 @@ test_c: buildtests_c $(Q) $(BINDIR)/$(CONFIG)/handshake_server || ( echo test handshake_server failed ; exit 1 ) $(E) "[RUN] Testing handshake_server_with_readahead_handshaker" $(Q) $(BINDIR)/$(CONFIG)/handshake_server_with_readahead_handshaker || ( echo test handshake_server_with_readahead_handshaker failed ; exit 1 ) + $(E) "[RUN] Testing histogram_test" + $(Q) $(BINDIR)/$(CONFIG)/histogram_test || ( echo test histogram_test failed ; exit 1 ) $(E) "[RUN] Testing hpack_parser_test" $(Q) $(BINDIR)/$(CONFIG)/hpack_parser_test || ( echo test hpack_parser_test failed ; exit 1 ) $(E) "[RUN] Testing hpack_table_test" @@ -2829,7 +2829,6 @@ LIBGPR_SRC = \ src/core/lib/support/env_posix.cc \ src/core/lib/support/env_windows.cc \ src/core/lib/support/fork.cc \ - src/core/lib/support/histogram.cc \ src/core/lib/support/host_port.cc \ src/core/lib/support/log.cc \ src/core/lib/support/log_android.cc \ @@ -2869,7 +2868,6 @@ PUBLIC_HEADERS_C += \ include/grpc/support/avl.h \ include/grpc/support/cmdline.h \ include/grpc/support/cpu.h \ - include/grpc/support/histogram.h \ include/grpc/support/host_port.h \ include/grpc/support/log.h \ include/grpc/support/log_windows.h \ @@ -3623,6 +3621,7 @@ LIBGRPC_TEST_UTIL_SRC = \ test/core/iomgr/endpoint_tests.cc \ test/core/util/debugger_macros.cc \ test/core/util/grpc_profiler.cc \ + test/core/util/histogram.cc \ test/core/util/memory_counters.cc \ test/core/util/mock_endpoint.cc \ test/core/util/parse_hexstring.cc \ @@ -3882,6 +3881,7 @@ LIBGRPC_TEST_UTIL_UNSECURE_SRC = \ test/core/iomgr/endpoint_tests.cc \ test/core/util/debugger_macros.cc \ test/core/util/grpc_profiler.cc \ + test/core/util/histogram.cc \ test/core/util/memory_counters.cc \ test/core/util/mock_endpoint.cc \ test/core/util/parse_hexstring.cc \ @@ -4591,7 +4591,6 @@ PUBLIC_HEADERS_CXX += \ include/grpc/support/avl.h \ include/grpc/support/cmdline.h \ include/grpc/support/cpu.h \ - include/grpc/support/histogram.h \ include/grpc/support/host_port.h \ include/grpc/support/log.h \ include/grpc/support/log_windows.h \ @@ -5079,7 +5078,6 @@ PUBLIC_HEADERS_CXX += \ include/grpc/support/avl.h \ include/grpc/support/cmdline.h \ include/grpc/support/cpu.h \ - include/grpc/support/histogram.h \ include/grpc/support/host_port.h \ include/grpc/support/log.h \ include/grpc/support/log_windows.h \ @@ -5801,7 +5799,6 @@ PUBLIC_HEADERS_CXX += \ include/grpc/support/avl.h \ include/grpc/support/cmdline.h \ include/grpc/support/cpu.h \ - include/grpc/support/histogram.h \ include/grpc/support/host_port.h \ include/grpc/support/log.h \ include/grpc/support/log_windows.h \ @@ -10095,38 +10092,6 @@ endif endif -GPR_HISTOGRAM_TEST_SRC = \ - test/core/support/histogram_test.cc \ - -GPR_HISTOGRAM_TEST_OBJS = $(addprefix $(OBJDIR)/$(CONFIG)/, $(addsuffix .o, $(basename $(GPR_HISTOGRAM_TEST_SRC)))) -ifeq ($(NO_SECURE),true) - -# You can't build secure targets if you don't have OpenSSL. - -$(BINDIR)/$(CONFIG)/gpr_histogram_test: openssl_dep_error - -else - - - -$(BINDIR)/$(CONFIG)/gpr_histogram_test: $(GPR_HISTOGRAM_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a - $(E) "[LD] Linking $@" - $(Q) mkdir -p `dirname $@` - $(Q) $(LD) $(LDFLAGS) $(GPR_HISTOGRAM_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a $(LDLIBS) $(LDLIBS_SECURE) -o $(BINDIR)/$(CONFIG)/gpr_histogram_test - -endif - -$(OBJDIR)/$(CONFIG)/test/core/support/histogram_test.o: $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a - -deps_gpr_histogram_test: $(GPR_HISTOGRAM_TEST_OBJS:.o=.dep) - -ifneq ($(NO_SECURE),true) -ifneq ($(NO_DEPS),true) --include $(GPR_HISTOGRAM_TEST_OBJS:.o=.dep) -endif -endif - - GPR_HOST_PORT_TEST_SRC = \ test/core/support/host_port_test.cc \ @@ -11157,6 +11122,38 @@ endif endif +HISTOGRAM_TEST_SRC = \ + test/core/util/histogram_test.cc \ + +HISTOGRAM_TEST_OBJS = $(addprefix $(OBJDIR)/$(CONFIG)/, $(addsuffix .o, $(basename $(HISTOGRAM_TEST_SRC)))) +ifeq ($(NO_SECURE),true) + +# You can't build secure targets if you don't have OpenSSL. + +$(BINDIR)/$(CONFIG)/histogram_test: openssl_dep_error + +else + + + +$(BINDIR)/$(CONFIG)/histogram_test: $(HISTOGRAM_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a + $(E) "[LD] Linking $@" + $(Q) mkdir -p `dirname $@` + $(Q) $(LD) $(LDFLAGS) $(HISTOGRAM_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a $(LDLIBS) $(LDLIBS_SECURE) -o $(BINDIR)/$(CONFIG)/histogram_test + +endif + +$(OBJDIR)/$(CONFIG)/test/core/util/histogram_test.o: $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a + +deps_histogram_test: $(HISTOGRAM_TEST_OBJS:.o=.dep) + +ifneq ($(NO_SECURE),true) +ifneq ($(NO_DEPS),true) +-include $(HISTOGRAM_TEST_OBJS:.o=.dep) +endif +endif + + HPACK_PARSER_FUZZER_TEST_SRC = \ test/core/transport/chttp2/hpack_parser_fuzzer_test.cc \ diff --git a/build.yaml b/build.yaml index 0908f7efd23..40c37dc53f7 100644 --- a/build.yaml +++ b/build.yaml @@ -41,7 +41,6 @@ filegroups: - src/core/lib/support/env_posix.cc - src/core/lib/support/env_windows.cc - src/core/lib/support/fork.cc - - src/core/lib/support/histogram.cc - src/core/lib/support/host_port.cc - src/core/lib/support/log.cc - src/core/lib/support/log_android.cc @@ -83,7 +82,6 @@ filegroups: - include/grpc/support/avl.h - include/grpc/support/cmdline.h - include/grpc/support/cpu.h - - include/grpc/support/histogram.h - include/grpc/support/host_port.h - include/grpc/support/log.h - include/grpc/support/log_windows.h @@ -714,6 +712,7 @@ filegroups: - test/core/iomgr/endpoint_tests.h - test/core/util/debugger_macros.h - test/core/util/grpc_profiler.h + - test/core/util/histogram.h - test/core/util/memory_counters.h - test/core/util/mock_endpoint.h - test/core/util/parse_hexstring.h @@ -731,6 +730,7 @@ filegroups: - test/core/iomgr/endpoint_tests.cc - test/core/util/debugger_macros.cc - test/core/util/grpc_profiler.cc + - test/core/util/histogram.cc - test/core/util/memory_counters.cc - test/core/util/mock_endpoint.cc - test/core/util/parse_hexstring.cc @@ -2196,15 +2196,6 @@ targets: - gpr_test_util - gpr uses_polling: false -- name: gpr_histogram_test - build: test - language: c - src: - - test/core/support/histogram_test.cc - deps: - - gpr_test_util - - gpr - uses_polling: false - name: gpr_host_port_test build: test language: c @@ -2553,6 +2544,15 @@ targets: platforms: - linux secure: true +- name: histogram_test + build: test + language: c + src: + - test/core/util/histogram_test.cc + deps: + - grpc_test_util + - gpr + uses_polling: false - name: hpack_parser_fuzzer_test build: fuzzer language: c diff --git a/config.m4 b/config.m4 index 6fe897f4d51..c026b83f359 100644 --- a/config.m4 +++ b/config.m4 @@ -54,7 +54,6 @@ if test "$PHP_GRPC" != "no"; then src/core/lib/support/env_posix.cc \ src/core/lib/support/env_windows.cc \ src/core/lib/support/fork.cc \ - src/core/lib/support/histogram.cc \ src/core/lib/support/host_port.cc \ src/core/lib/support/log.cc \ src/core/lib/support/log_android.cc \ diff --git a/config.w32 b/config.w32 index c2a4327c729..cd3a16a4653 100644 --- a/config.w32 +++ b/config.w32 @@ -31,7 +31,6 @@ if (PHP_GRPC != "no") { "src\\core\\lib\\support\\env_posix.cc " + "src\\core\\lib\\support\\env_windows.cc " + "src\\core\\lib\\support\\fork.cc " + - "src\\core\\lib\\support\\histogram.cc " + "src\\core\\lib\\support\\host_port.cc " + "src\\core\\lib\\support\\log.cc " + "src\\core\\lib\\support\\log_android.cc " + diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index 82dda2017fb..39b848414ad 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -113,7 +113,6 @@ Pod::Spec.new do |s| 'include/grpc/support/avl.h', 'include/grpc/support/cmdline.h', 'include/grpc/support/cpu.h', - 'include/grpc/support/histogram.h', 'include/grpc/support/host_port.h', 'include/grpc/support/log.h', 'include/grpc/support/log_windows.h', @@ -223,7 +222,6 @@ Pod::Spec.new do |s| 'src/core/lib/support/env_posix.cc', 'src/core/lib/support/env_windows.cc', 'src/core/lib/support/fork.cc', - 'src/core/lib/support/histogram.cc', 'src/core/lib/support/host_port.cc', 'src/core/lib/support/log.cc', 'src/core/lib/support/log_android.cc', diff --git a/grpc.def b/grpc.def index 07c0b3e928d..d4a18ccefc1 100644 --- a/grpc.def +++ b/grpc.def @@ -200,21 +200,6 @@ EXPORTS gpr_cmdline_usage_string gpr_cpu_num_cores gpr_cpu_current_cpu - gpr_histogram_create - gpr_histogram_destroy - gpr_histogram_add - gpr_histogram_merge - gpr_histogram_percentile - gpr_histogram_mean - gpr_histogram_stddev - gpr_histogram_variance - gpr_histogram_maximum - gpr_histogram_minimum - gpr_histogram_count - gpr_histogram_sum - gpr_histogram_sum_of_squares - gpr_histogram_get_contents - gpr_histogram_merge_contents gpr_join_host_port gpr_split_host_port gpr_log_severity_string diff --git a/grpc.gemspec b/grpc.gemspec index e553b770e43..d1859952619 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -52,7 +52,6 @@ Gem::Specification.new do |s| s.files += %w( include/grpc/support/avl.h ) s.files += %w( include/grpc/support/cmdline.h ) s.files += %w( include/grpc/support/cpu.h ) - s.files += %w( include/grpc/support/histogram.h ) s.files += %w( include/grpc/support/host_port.h ) s.files += %w( include/grpc/support/log.h ) s.files += %w( include/grpc/support/log_windows.h ) @@ -117,7 +116,6 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/support/env_posix.cc ) s.files += %w( src/core/lib/support/env_windows.cc ) s.files += %w( src/core/lib/support/fork.cc ) - s.files += %w( src/core/lib/support/histogram.cc ) s.files += %w( src/core/lib/support/host_port.cc ) s.files += %w( src/core/lib/support/log.cc ) s.files += %w( src/core/lib/support/log_android.cc ) diff --git a/grpc.gyp b/grpc.gyp index f2033b47b0b..4ceb480282c 100644 --- a/grpc.gyp +++ b/grpc.gyp @@ -173,7 +173,6 @@ 'src/core/lib/support/env_posix.cc', 'src/core/lib/support/env_windows.cc', 'src/core/lib/support/fork.cc', - 'src/core/lib/support/histogram.cc', 'src/core/lib/support/host_port.cc', 'src/core/lib/support/log.cc', 'src/core/lib/support/log_android.cc', @@ -506,6 +505,7 @@ 'test/core/iomgr/endpoint_tests.cc', 'test/core/util/debugger_macros.cc', 'test/core/util/grpc_profiler.cc', + 'test/core/util/histogram.cc', 'test/core/util/memory_counters.cc', 'test/core/util/mock_endpoint.cc', 'test/core/util/parse_hexstring.cc', @@ -716,6 +716,7 @@ 'test/core/iomgr/endpoint_tests.cc', 'test/core/util/debugger_macros.cc', 'test/core/util/grpc_profiler.cc', + 'test/core/util/histogram.cc', 'test/core/util/memory_counters.cc', 'test/core/util/mock_endpoint.cc', 'test/core/util/parse_hexstring.cc', diff --git a/include/grpc/module.modulemap b/include/grpc/module.modulemap index 0faa448b70f..67136cba8ad 100644 --- a/include/grpc/module.modulemap +++ b/include/grpc/module.modulemap @@ -7,7 +7,6 @@ framework module grpc { header "support/avl.h" header "support/cmdline.h" header "support/cpu.h" - header "support/histogram.h" header "support/host_port.h" header "support/log.h" header "support/log_windows.h" diff --git a/include/grpc/support/histogram.h b/include/grpc/support/histogram.h deleted file mode 100644 index d2794d847e8..00000000000 --- a/include/grpc/support/histogram.h +++ /dev/null @@ -1,64 +0,0 @@ -/* - * - * Copyright 2015 gRPC authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ - -#ifndef GRPC_SUPPORT_HISTOGRAM_H -#define GRPC_SUPPORT_HISTOGRAM_H - -#include -#include - -#ifdef __cplusplus -extern "C" { -#endif - -typedef struct gpr_histogram gpr_histogram; - -GPRAPI gpr_histogram* gpr_histogram_create(double resolution, - double max_bucket_start); -GPRAPI void gpr_histogram_destroy(gpr_histogram* h); -GPRAPI void gpr_histogram_add(gpr_histogram* h, double x); - -/** The following merges the second histogram into the first. It only works - if they have the same buckets and resolution. Returns 0 on failure, 1 - on success */ -GPRAPI int gpr_histogram_merge(gpr_histogram* dst, const gpr_histogram* src); - -GPRAPI double gpr_histogram_percentile(gpr_histogram* histogram, - double percentile); -GPRAPI double gpr_histogram_mean(gpr_histogram* histogram); -GPRAPI double gpr_histogram_stddev(gpr_histogram* histogram); -GPRAPI double gpr_histogram_variance(gpr_histogram* histogram); -GPRAPI double gpr_histogram_maximum(gpr_histogram* histogram); -GPRAPI double gpr_histogram_minimum(gpr_histogram* histogram); -GPRAPI double gpr_histogram_count(gpr_histogram* histogram); -GPRAPI double gpr_histogram_sum(gpr_histogram* histogram); -GPRAPI double gpr_histogram_sum_of_squares(gpr_histogram* histogram); - -GPRAPI const uint32_t* gpr_histogram_get_contents(gpr_histogram* histogram, - size_t* count); -GPRAPI void gpr_histogram_merge_contents(gpr_histogram* histogram, - const uint32_t* data, - size_t data_count, double min_seen, - double max_seen, double sum, - double sum_of_squares, double count); - -#ifdef __cplusplus -} -#endif - -#endif /* GRPC_SUPPORT_HISTOGRAM_H */ diff --git a/package.xml b/package.xml index 435fb4c2c26..b4d8c886930 100644 --- a/package.xml +++ b/package.xml @@ -64,7 +64,6 @@ - @@ -129,7 +128,6 @@ - diff --git a/src/python/grpcio/grpc_core_dependencies.py b/src/python/grpcio/grpc_core_dependencies.py index 56d6ebd8425..d2a68f0902f 100644 --- a/src/python/grpcio/grpc_core_dependencies.py +++ b/src/python/grpcio/grpc_core_dependencies.py @@ -30,7 +30,6 @@ CORE_SOURCE_FILES = [ 'src/core/lib/support/env_posix.cc', 'src/core/lib/support/env_windows.cc', 'src/core/lib/support/fork.cc', - 'src/core/lib/support/histogram.cc', 'src/core/lib/support/host_port.cc', 'src/core/lib/support/log.cc', 'src/core/lib/support/log_android.cc', diff --git a/src/ruby/ext/grpc/rb_grpc_imports.generated.c b/src/ruby/ext/grpc/rb_grpc_imports.generated.c index 648d515003a..56f1d4c93f4 100644 --- a/src/ruby/ext/grpc/rb_grpc_imports.generated.c +++ b/src/ruby/ext/grpc/rb_grpc_imports.generated.c @@ -223,21 +223,6 @@ gpr_cmdline_destroy_type gpr_cmdline_destroy_import; gpr_cmdline_usage_string_type gpr_cmdline_usage_string_import; gpr_cpu_num_cores_type gpr_cpu_num_cores_import; gpr_cpu_current_cpu_type gpr_cpu_current_cpu_import; -gpr_histogram_create_type gpr_histogram_create_import; -gpr_histogram_destroy_type gpr_histogram_destroy_import; -gpr_histogram_add_type gpr_histogram_add_import; -gpr_histogram_merge_type gpr_histogram_merge_import; -gpr_histogram_percentile_type gpr_histogram_percentile_import; -gpr_histogram_mean_type gpr_histogram_mean_import; -gpr_histogram_stddev_type gpr_histogram_stddev_import; -gpr_histogram_variance_type gpr_histogram_variance_import; -gpr_histogram_maximum_type gpr_histogram_maximum_import; -gpr_histogram_minimum_type gpr_histogram_minimum_import; -gpr_histogram_count_type gpr_histogram_count_import; -gpr_histogram_sum_type gpr_histogram_sum_import; -gpr_histogram_sum_of_squares_type gpr_histogram_sum_of_squares_import; -gpr_histogram_get_contents_type gpr_histogram_get_contents_import; -gpr_histogram_merge_contents_type gpr_histogram_merge_contents_import; gpr_join_host_port_type gpr_join_host_port_import; gpr_split_host_port_type gpr_split_host_port_import; gpr_log_severity_string_type gpr_log_severity_string_import; @@ -510,21 +495,6 @@ void grpc_rb_load_imports(HMODULE library) { gpr_cmdline_usage_string_import = (gpr_cmdline_usage_string_type) GetProcAddress(library, "gpr_cmdline_usage_string"); gpr_cpu_num_cores_import = (gpr_cpu_num_cores_type) GetProcAddress(library, "gpr_cpu_num_cores"); gpr_cpu_current_cpu_import = (gpr_cpu_current_cpu_type) GetProcAddress(library, "gpr_cpu_current_cpu"); - gpr_histogram_create_import = (gpr_histogram_create_type) GetProcAddress(library, "gpr_histogram_create"); - gpr_histogram_destroy_import = (gpr_histogram_destroy_type) GetProcAddress(library, "gpr_histogram_destroy"); - gpr_histogram_add_import = (gpr_histogram_add_type) GetProcAddress(library, "gpr_histogram_add"); - gpr_histogram_merge_import = (gpr_histogram_merge_type) GetProcAddress(library, "gpr_histogram_merge"); - gpr_histogram_percentile_import = (gpr_histogram_percentile_type) GetProcAddress(library, "gpr_histogram_percentile"); - gpr_histogram_mean_import = (gpr_histogram_mean_type) GetProcAddress(library, "gpr_histogram_mean"); - gpr_histogram_stddev_import = (gpr_histogram_stddev_type) GetProcAddress(library, "gpr_histogram_stddev"); - gpr_histogram_variance_import = (gpr_histogram_variance_type) GetProcAddress(library, "gpr_histogram_variance"); - gpr_histogram_maximum_import = (gpr_histogram_maximum_type) GetProcAddress(library, "gpr_histogram_maximum"); - gpr_histogram_minimum_import = (gpr_histogram_minimum_type) GetProcAddress(library, "gpr_histogram_minimum"); - gpr_histogram_count_import = (gpr_histogram_count_type) GetProcAddress(library, "gpr_histogram_count"); - gpr_histogram_sum_import = (gpr_histogram_sum_type) GetProcAddress(library, "gpr_histogram_sum"); - gpr_histogram_sum_of_squares_import = (gpr_histogram_sum_of_squares_type) GetProcAddress(library, "gpr_histogram_sum_of_squares"); - gpr_histogram_get_contents_import = (gpr_histogram_get_contents_type) GetProcAddress(library, "gpr_histogram_get_contents"); - gpr_histogram_merge_contents_import = (gpr_histogram_merge_contents_type) GetProcAddress(library, "gpr_histogram_merge_contents"); gpr_join_host_port_import = (gpr_join_host_port_type) GetProcAddress(library, "gpr_join_host_port"); gpr_split_host_port_import = (gpr_split_host_port_type) GetProcAddress(library, "gpr_split_host_port"); gpr_log_severity_string_import = (gpr_log_severity_string_type) GetProcAddress(library, "gpr_log_severity_string"); diff --git a/src/ruby/ext/grpc/rb_grpc_imports.generated.h b/src/ruby/ext/grpc/rb_grpc_imports.generated.h index c2698d16ea4..62223fda5b7 100644 --- a/src/ruby/ext/grpc/rb_grpc_imports.generated.h +++ b/src/ruby/ext/grpc/rb_grpc_imports.generated.h @@ -36,7 +36,6 @@ #include #include #include -#include #include #include #include @@ -649,51 +648,6 @@ extern gpr_cpu_num_cores_type gpr_cpu_num_cores_import; typedef unsigned(*gpr_cpu_current_cpu_type)(void); extern gpr_cpu_current_cpu_type gpr_cpu_current_cpu_import; #define gpr_cpu_current_cpu gpr_cpu_current_cpu_import -typedef gpr_histogram*(*gpr_histogram_create_type)(double resolution, double max_bucket_start); -extern gpr_histogram_create_type gpr_histogram_create_import; -#define gpr_histogram_create gpr_histogram_create_import -typedef void(*gpr_histogram_destroy_type)(gpr_histogram* h); -extern gpr_histogram_destroy_type gpr_histogram_destroy_import; -#define gpr_histogram_destroy gpr_histogram_destroy_import -typedef void(*gpr_histogram_add_type)(gpr_histogram* h, double x); -extern gpr_histogram_add_type gpr_histogram_add_import; -#define gpr_histogram_add gpr_histogram_add_import -typedef int(*gpr_histogram_merge_type)(gpr_histogram* dst, const gpr_histogram* src); -extern gpr_histogram_merge_type gpr_histogram_merge_import; -#define gpr_histogram_merge gpr_histogram_merge_import -typedef double(*gpr_histogram_percentile_type)(gpr_histogram* histogram, double percentile); -extern gpr_histogram_percentile_type gpr_histogram_percentile_import; -#define gpr_histogram_percentile gpr_histogram_percentile_import -typedef double(*gpr_histogram_mean_type)(gpr_histogram* histogram); -extern gpr_histogram_mean_type gpr_histogram_mean_import; -#define gpr_histogram_mean gpr_histogram_mean_import -typedef double(*gpr_histogram_stddev_type)(gpr_histogram* histogram); -extern gpr_histogram_stddev_type gpr_histogram_stddev_import; -#define gpr_histogram_stddev gpr_histogram_stddev_import -typedef double(*gpr_histogram_variance_type)(gpr_histogram* histogram); -extern gpr_histogram_variance_type gpr_histogram_variance_import; -#define gpr_histogram_variance gpr_histogram_variance_import -typedef double(*gpr_histogram_maximum_type)(gpr_histogram* histogram); -extern gpr_histogram_maximum_type gpr_histogram_maximum_import; -#define gpr_histogram_maximum gpr_histogram_maximum_import -typedef double(*gpr_histogram_minimum_type)(gpr_histogram* histogram); -extern gpr_histogram_minimum_type gpr_histogram_minimum_import; -#define gpr_histogram_minimum gpr_histogram_minimum_import -typedef double(*gpr_histogram_count_type)(gpr_histogram* histogram); -extern gpr_histogram_count_type gpr_histogram_count_import; -#define gpr_histogram_count gpr_histogram_count_import -typedef double(*gpr_histogram_sum_type)(gpr_histogram* histogram); -extern gpr_histogram_sum_type gpr_histogram_sum_import; -#define gpr_histogram_sum gpr_histogram_sum_import -typedef double(*gpr_histogram_sum_of_squares_type)(gpr_histogram* histogram); -extern gpr_histogram_sum_of_squares_type gpr_histogram_sum_of_squares_import; -#define gpr_histogram_sum_of_squares gpr_histogram_sum_of_squares_import -typedef const uint32_t*(*gpr_histogram_get_contents_type)(gpr_histogram* histogram, size_t* count); -extern gpr_histogram_get_contents_type gpr_histogram_get_contents_import; -#define gpr_histogram_get_contents gpr_histogram_get_contents_import -typedef void(*gpr_histogram_merge_contents_type)(gpr_histogram* histogram, const uint32_t* data, size_t data_count, double min_seen, double max_seen, double sum, double sum_of_squares, double count); -extern gpr_histogram_merge_contents_type gpr_histogram_merge_contents_import; -#define gpr_histogram_merge_contents gpr_histogram_merge_contents_import typedef int(*gpr_join_host_port_type)(char** out, const char* host, int port); extern gpr_join_host_port_type gpr_join_host_port_import; #define gpr_join_host_port gpr_join_host_port_import diff --git a/test/core/fling/client.cc b/test/core/fling/client.cc index 544b66d4808..69fb6dc7c71 100644 --- a/test/core/fling/client.cc +++ b/test/core/fling/client.cc @@ -22,15 +22,15 @@ #include #include -#include #include #include #include #include "src/core/lib/profiling/timers.h" #include "test/core/util/grpc_profiler.h" +#include "test/core/util/histogram.h" #include "test/core/util/test_config.h" -static gpr_histogram* histogram; +static grpc_histogram* histogram; static grpc_byte_buffer* the_buffer; static grpc_channel* channel; static grpc_completion_queue* cq; @@ -195,7 +195,7 @@ int main(int argc, char** argv) { channel = grpc_insecure_channel_create(target, nullptr, nullptr); cq = grpc_completion_queue_create_for_next(nullptr); the_buffer = grpc_raw_byte_buffer_create(&slice, (size_t)payload_size); - histogram = gpr_histogram_create(0.01, 60e9); + histogram = grpc_histogram_create(0.01, 60e9); sc.init(); @@ -213,7 +213,7 @@ int main(int argc, char** argv) { start = now(); sc.do_one_step(); stop = now(); - gpr_histogram_add(histogram, stop - start); + grpc_histogram_add(histogram, stop - start); } grpc_profiler_stop(); @@ -232,11 +232,11 @@ int main(int argc, char** argv) { grpc_slice_unref(slice); gpr_log(GPR_INFO, "latency (50/95/99/99.9): %f/%f/%f/%f", - gpr_histogram_percentile(histogram, 50), - gpr_histogram_percentile(histogram, 95), - gpr_histogram_percentile(histogram, 99), - gpr_histogram_percentile(histogram, 99.9)); - gpr_histogram_destroy(histogram); + grpc_histogram_percentile(histogram, 50), + grpc_histogram_percentile(histogram, 95), + grpc_histogram_percentile(histogram, 99), + grpc_histogram_percentile(histogram, 99.9)); + grpc_histogram_destroy(histogram); grpc_shutdown(); diff --git a/test/core/network_benchmarks/low_level_ping_pong.cc b/test/core/network_benchmarks/low_level_ping_pong.cc index 687395d9165..2ae9a45d7c6 100644 --- a/test/core/network_benchmarks/low_level_ping_pong.cc +++ b/test/core/network_benchmarks/low_level_ping_pong.cc @@ -36,13 +36,13 @@ #include #include -#include #include #include #include #include #include "src/core/lib/iomgr/error.h" #include "src/core/lib/iomgr/socket_utils_posix.h" +#include "test/core/util/histogram.h" typedef struct fd_pair { int read_fd; @@ -275,14 +275,14 @@ static void server_thread_wrap(void* arg) { server_thread(args); } -static void print_histogram(gpr_histogram* histogram) { +static void print_histogram(grpc_histogram* histogram) { /* TODO(klempner): Print more detailed information, such as detailed histogram buckets */ gpr_log(GPR_INFO, "latency (50/95/99/99.9): %f/%f/%f/%f", - gpr_histogram_percentile(histogram, 50), - gpr_histogram_percentile(histogram, 95), - gpr_histogram_percentile(histogram, 99), - gpr_histogram_percentile(histogram, 99.9)); + grpc_histogram_percentile(histogram, 50), + grpc_histogram_percentile(histogram, 95), + grpc_histogram_percentile(histogram, 99), + grpc_histogram_percentile(histogram, 99.9)); } static double now(void) { @@ -293,7 +293,7 @@ static double now(void) { static void client_thread(thread_args* args) { char* buf = static_cast(gpr_malloc(args->msg_size * sizeof(char))); memset(buf, 0, args->msg_size * sizeof(char)); - gpr_histogram* histogram = gpr_histogram_create(0.01, 60e9); + grpc_histogram* histogram = grpc_histogram_create(0.01, 60e9); double start_time; double end_time; double interval; @@ -316,13 +316,13 @@ static void client_thread(thread_args* args) { end_time = now(); if (i > kNumIters / 2) { interval = end_time - start_time; - gpr_histogram_add(histogram, interval); + grpc_histogram_add(histogram, interval); } } print_histogram(histogram); error: gpr_free(buf); - gpr_histogram_destroy(histogram); + grpc_histogram_destroy(histogram); } /* This roughly matches tcp_server's create_listening_socket */ diff --git a/test/core/support/BUILD b/test/core/support/BUILD index 32b64d4b8e1..4372b49b545 100644 --- a/test/core/support/BUILD +++ b/test/core/support/BUILD @@ -68,16 +68,6 @@ grpc_cc_test( ], ) -grpc_cc_test( - name = "histogram_test", - srcs = ["histogram_test.cc"], - language = "C++", - deps = [ - "//:gpr", - "//test/core/util:gpr_test_util", - ], -) - grpc_cc_test( name = "host_port_test", srcs = ["host_port_test.cc"], diff --git a/test/core/support/histogram_test.cc b/test/core/support/histogram_test.cc deleted file mode 100644 index 86b7d599e6a..00000000000 --- a/test/core/support/histogram_test.cc +++ /dev/null @@ -1,163 +0,0 @@ -/* - * - * Copyright 2015 gRPC authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ - -#include -#include - -#define LOG_TEST(x) gpr_log(GPR_INFO, "%s", x); - -static void test_no_op(void) { - gpr_histogram_destroy(gpr_histogram_create(0.01, 60e9)); -} - -static void expect_percentile(gpr_histogram* h, double percentile, - double min_expect, double max_expect) { - double got = gpr_histogram_percentile(h, percentile); - gpr_log(GPR_INFO, "@%f%%, expect %f <= %f <= %f", percentile, min_expect, got, - max_expect); - GPR_ASSERT(min_expect <= got); - GPR_ASSERT(got <= max_expect); -} - -static void test_simple(void) { - gpr_histogram* h; - - LOG_TEST("test_simple"); - - h = gpr_histogram_create(0.01, 60e9); - gpr_histogram_add(h, 10000); - gpr_histogram_add(h, 10000); - gpr_histogram_add(h, 11000); - gpr_histogram_add(h, 11000); - - expect_percentile(h, 50, 10001, 10999); - GPR_ASSERT(gpr_histogram_mean(h) == 10500); - - gpr_histogram_destroy(h); -} - -static void test_percentile(void) { - gpr_histogram* h; - double last; - double i; - double cur; - - LOG_TEST("test_percentile"); - - h = gpr_histogram_create(0.05, 1e9); - gpr_histogram_add(h, 2.5); - gpr_histogram_add(h, 2.5); - gpr_histogram_add(h, 8); - gpr_histogram_add(h, 4); - - GPR_ASSERT(gpr_histogram_count(h) == 4); - GPR_ASSERT(gpr_histogram_minimum(h) == 2.5); - GPR_ASSERT(gpr_histogram_maximum(h) == 8); - GPR_ASSERT(gpr_histogram_sum(h) == 17); - GPR_ASSERT(gpr_histogram_sum_of_squares(h) == 92.5); - GPR_ASSERT(gpr_histogram_mean(h) == 4.25); - GPR_ASSERT(gpr_histogram_variance(h) == 5.0625); - GPR_ASSERT(gpr_histogram_stddev(h) == 2.25); - - expect_percentile(h, -10, 2.5, 2.5); - expect_percentile(h, 0, 2.5, 2.5); - expect_percentile(h, 12.5, 2.5, 2.5); - expect_percentile(h, 25, 2.5, 2.5); - expect_percentile(h, 37.5, 2.5, 2.8); - expect_percentile(h, 50, 3.0, 3.5); - expect_percentile(h, 62.5, 3.5, 4.5); - expect_percentile(h, 75, 5, 7.9); - expect_percentile(h, 100, 8, 8); - expect_percentile(h, 110, 8, 8); - - /* test monotonicity */ - last = 0.0; - for (i = 0; i < 100.0; i += 0.01) { - cur = gpr_histogram_percentile(h, i); - GPR_ASSERT(cur >= last); - last = cur; - } - - gpr_histogram_destroy(h); -} - -static void test_merge(void) { - gpr_histogram *h1, *h2; - double last; - double i; - double cur; - - LOG_TEST("test_merge"); - - h1 = gpr_histogram_create(0.05, 1e9); - gpr_histogram_add(h1, 2.5); - gpr_histogram_add(h1, 2.5); - gpr_histogram_add(h1, 8); - gpr_histogram_add(h1, 4); - - h2 = gpr_histogram_create(0.01, 1e9); - GPR_ASSERT(gpr_histogram_merge(h1, h2) == 0); - gpr_histogram_destroy(h2); - - h2 = gpr_histogram_create(0.05, 1e10); - GPR_ASSERT(gpr_histogram_merge(h1, h2) == 0); - gpr_histogram_destroy(h2); - - h2 = gpr_histogram_create(0.05, 1e9); - GPR_ASSERT(gpr_histogram_merge(h1, h2) == 1); - GPR_ASSERT(gpr_histogram_count(h1) == 4); - GPR_ASSERT(gpr_histogram_minimum(h1) == 2.5); - GPR_ASSERT(gpr_histogram_maximum(h1) == 8); - GPR_ASSERT(gpr_histogram_sum(h1) == 17); - GPR_ASSERT(gpr_histogram_sum_of_squares(h1) == 92.5); - GPR_ASSERT(gpr_histogram_mean(h1) == 4.25); - GPR_ASSERT(gpr_histogram_variance(h1) == 5.0625); - GPR_ASSERT(gpr_histogram_stddev(h1) == 2.25); - gpr_histogram_destroy(h2); - - h2 = gpr_histogram_create(0.05, 1e9); - gpr_histogram_add(h2, 7.0); - gpr_histogram_add(h2, 17.0); - gpr_histogram_add(h2, 1.0); - GPR_ASSERT(gpr_histogram_merge(h1, h2) == 1); - GPR_ASSERT(gpr_histogram_count(h1) == 7); - GPR_ASSERT(gpr_histogram_minimum(h1) == 1.0); - GPR_ASSERT(gpr_histogram_maximum(h1) == 17.0); - GPR_ASSERT(gpr_histogram_sum(h1) == 42.0); - GPR_ASSERT(gpr_histogram_sum_of_squares(h1) == 431.5); - GPR_ASSERT(gpr_histogram_mean(h1) == 6.0); - - /* test monotonicity */ - last = 0.0; - for (i = 0; i < 100.0; i += 0.01) { - cur = gpr_histogram_percentile(h1, i); - GPR_ASSERT(cur >= last); - last = cur; - } - - gpr_histogram_destroy(h1); - gpr_histogram_destroy(h2); -} - -int main(void) { - test_no_op(); - test_simple(); - test_percentile(); - test_merge(); - return 0; -} diff --git a/test/core/surface/public_headers_must_be_c89.c b/test/core/surface/public_headers_must_be_c89.c index 33dc70a6855..8d2384ba61f 100644 --- a/test/core/surface/public_headers_must_be_c89.c +++ b/test/core/surface/public_headers_must_be_c89.c @@ -50,7 +50,6 @@ #include #include #include -#include #include #include #include @@ -266,21 +265,6 @@ int main(int argc, char **argv) { printf("%lx", (unsigned long) gpr_cmdline_usage_string); printf("%lx", (unsigned long) gpr_cpu_num_cores); printf("%lx", (unsigned long) gpr_cpu_current_cpu); - printf("%lx", (unsigned long) gpr_histogram_create); - printf("%lx", (unsigned long) gpr_histogram_destroy); - printf("%lx", (unsigned long) gpr_histogram_add); - printf("%lx", (unsigned long) gpr_histogram_merge); - printf("%lx", (unsigned long) gpr_histogram_percentile); - printf("%lx", (unsigned long) gpr_histogram_mean); - printf("%lx", (unsigned long) gpr_histogram_stddev); - printf("%lx", (unsigned long) gpr_histogram_variance); - printf("%lx", (unsigned long) gpr_histogram_maximum); - printf("%lx", (unsigned long) gpr_histogram_minimum); - printf("%lx", (unsigned long) gpr_histogram_count); - printf("%lx", (unsigned long) gpr_histogram_sum); - printf("%lx", (unsigned long) gpr_histogram_sum_of_squares); - printf("%lx", (unsigned long) gpr_histogram_get_contents); - printf("%lx", (unsigned long) gpr_histogram_merge_contents); printf("%lx", (unsigned long) gpr_join_host_port); printf("%lx", (unsigned long) gpr_split_host_port); printf("%lx", (unsigned long) gpr_log_severity_string); diff --git a/test/core/util/BUILD b/test/core/util/BUILD index 6443553466f..268547f6c9d 100644 --- a/test/core/util/BUILD +++ b/test/core/util/BUILD @@ -16,7 +16,10 @@ load("//bazel:grpc_build_system.bzl", "grpc_cc_library", "grpc_cc_test", "grpc_c licenses(["notice"]) # Apache v2 -grpc_package(name = "test/core/util", visibility = "public") +grpc_package( + name = "test/core/util", + visibility = "public", +) grpc_cc_library( name = "gpr_test_util", @@ -49,6 +52,7 @@ grpc_cc_library( name = "grpc_test_util_base", srcs = [ "grpc_profiler.cc", + "histogram.cc", "mock_endpoint.cc", "parse_hexstring.cc", "passthru_endpoint.cc", @@ -62,6 +66,7 @@ grpc_cc_library( ], hdrs = [ "grpc_profiler.h", + "histogram.h", "mock_endpoint.h", "parse_hexstring.h", "passthru_endpoint.h", @@ -76,8 +81,8 @@ grpc_cc_library( language = "C++", deps = [ ":gpr_test_util", + ":grpc_debugger_macros", "//:grpc_common", - ":grpc_debugger_macros" ], ) @@ -107,13 +112,23 @@ grpc_cc_library( name = "fuzzer_corpus_test", testonly = 1, srcs = ["fuzzer_corpus_test.cc"], + external_deps = [ + "gtest", + "gflags", + ], deps = [ ":gpr_test_util", "//:grpc", ], - external_deps = [ - "gtest", - "gflags", +) + +grpc_cc_test( + name = "histogram_test", + srcs = ["histogram_test.cc"], + language = "C++", + deps = [ + ":grpc_test_util", + "//:gpr", ], ) diff --git a/src/core/lib/support/histogram.cc b/test/core/util/histogram.cc similarity index 72% rename from src/core/lib/support/histogram.cc rename to test/core/util/histogram.cc index 73c821a28b0..2f916f831d6 100644 --- a/src/core/lib/support/histogram.cc +++ b/test/core/util/histogram.cc @@ -16,8 +16,6 @@ * */ -#include - #include #include #include @@ -27,12 +25,14 @@ #include #include +#include "test/core/util/histogram.h" + /* Histograms are stored with exponentially increasing bucket sizes. The first bucket is [0, m) where m = 1 + resolution Bucket n (n>=1) contains [m**n, m**(n+1)) There are sufficient buckets to reach max_bucket_start */ -struct gpr_histogram { +struct grpc_histogram { /* Sum of all values seen so far */ double sum; /* Sum of squares of all values seen so far */ @@ -55,25 +55,25 @@ struct gpr_histogram { }; /* determine a bucket index given a value - does no bounds checking */ -static size_t bucket_for_unchecked(gpr_histogram* h, double x) { +static size_t bucket_for_unchecked(grpc_histogram* h, double x) { return (size_t)(log(x) * h->one_on_log_multiplier); } /* bounds checked version of the above */ -static size_t bucket_for(gpr_histogram* h, double x) { +static size_t bucket_for(grpc_histogram* h, double x) { size_t bucket = bucket_for_unchecked(h, GPR_CLAMP(x, 1.0, h->max_possible)); GPR_ASSERT(bucket < h->num_buckets); return bucket; } /* at what value does a bucket start? */ -static double bucket_start(gpr_histogram* h, double x) { +static double bucket_start(grpc_histogram* h, double x) { return pow(h->multiplier, x); } -gpr_histogram* gpr_histogram_create(double resolution, - double max_bucket_start) { - gpr_histogram* h = (gpr_histogram*)gpr_malloc(sizeof(gpr_histogram)); +grpc_histogram* grpc_histogram_create(double resolution, + double max_bucket_start) { + grpc_histogram* h = (grpc_histogram*)gpr_malloc(sizeof(grpc_histogram)); GPR_ASSERT(resolution > 0.0); GPR_ASSERT(max_bucket_start > resolution); h->sum = 0.0; @@ -91,12 +91,12 @@ gpr_histogram* gpr_histogram_create(double resolution, return h; } -void gpr_histogram_destroy(gpr_histogram* h) { +void grpc_histogram_destroy(grpc_histogram* h) { gpr_free(h->buckets); gpr_free(h); } -void gpr_histogram_add(gpr_histogram* h, double x) { +void grpc_histogram_add(grpc_histogram* h, double x) { h->sum += x; h->sum_of_squares += x * x; h->count++; @@ -109,22 +109,22 @@ void gpr_histogram_add(gpr_histogram* h, double x) { h->buckets[bucket_for(h, x)]++; } -int gpr_histogram_merge(gpr_histogram* dst, const gpr_histogram* src) { +int grpc_histogram_merge(grpc_histogram* dst, const grpc_histogram* src) { if ((dst->num_buckets != src->num_buckets) || (dst->multiplier != src->multiplier)) { /* Fail because these histograms don't match */ return 0; } - gpr_histogram_merge_contents(dst, src->buckets, src->num_buckets, - src->min_seen, src->max_seen, src->sum, - src->sum_of_squares, src->count); + grpc_histogram_merge_contents(dst, src->buckets, src->num_buckets, + src->min_seen, src->max_seen, src->sum, + src->sum_of_squares, src->count); return 1; } -void gpr_histogram_merge_contents(gpr_histogram* dst, const uint32_t* data, - size_t data_count, double min_seen, - double max_seen, double sum, - double sum_of_squares, double count) { +void grpc_histogram_merge_contents(grpc_histogram* dst, const uint32_t* data, + size_t data_count, double min_seen, + double max_seen, double sum, + double sum_of_squares, double count) { size_t i; GPR_ASSERT(dst->num_buckets == data_count); dst->sum += sum; @@ -141,7 +141,7 @@ void gpr_histogram_merge_contents(gpr_histogram* dst, const uint32_t* data, } } -static double threshold_for_count_below(gpr_histogram* h, double count_below) { +static double threshold_for_count_below(grpc_histogram* h, double count_below) { double count_so_far; double lower_bound; double upper_bound; @@ -190,38 +190,38 @@ static double threshold_for_count_below(gpr_histogram* h, double count_below) { } } -double gpr_histogram_percentile(gpr_histogram* h, double percentile) { +double grpc_histogram_percentile(grpc_histogram* h, double percentile) { return threshold_for_count_below(h, h->count * percentile / 100.0); } -double gpr_histogram_mean(gpr_histogram* h) { +double grpc_histogram_mean(grpc_histogram* h) { GPR_ASSERT(h->count != 0); return h->sum / h->count; } -double gpr_histogram_stddev(gpr_histogram* h) { - return sqrt(gpr_histogram_variance(h)); +double grpc_histogram_stddev(grpc_histogram* h) { + return sqrt(grpc_histogram_variance(h)); } -double gpr_histogram_variance(gpr_histogram* h) { +double grpc_histogram_variance(grpc_histogram* h) { if (h->count == 0) return 0.0; return (h->sum_of_squares * h->count - h->sum * h->sum) / (h->count * h->count); } -double gpr_histogram_maximum(gpr_histogram* h) { return h->max_seen; } +double grpc_histogram_maximum(grpc_histogram* h) { return h->max_seen; } -double gpr_histogram_minimum(gpr_histogram* h) { return h->min_seen; } +double grpc_histogram_minimum(grpc_histogram* h) { return h->min_seen; } -double gpr_histogram_count(gpr_histogram* h) { return h->count; } +double grpc_histogram_count(grpc_histogram* h) { return h->count; } -double gpr_histogram_sum(gpr_histogram* h) { return h->sum; } +double grpc_histogram_sum(grpc_histogram* h) { return h->sum; } -double gpr_histogram_sum_of_squares(gpr_histogram* h) { +double grpc_histogram_sum_of_squares(grpc_histogram* h) { return h->sum_of_squares; } -const uint32_t* gpr_histogram_get_contents(gpr_histogram* h, size_t* size) { +const uint32_t* grpc_histogram_get_contents(grpc_histogram* h, size_t* size) { *size = h->num_buckets; return h->buckets; } diff --git a/test/core/util/histogram.h b/test/core/util/histogram.h new file mode 100644 index 00000000000..9d4985e64fb --- /dev/null +++ b/test/core/util/histogram.h @@ -0,0 +1,62 @@ +/* + * + * Copyright 2015 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +#ifndef GRPC_SUPPORT_HISTOGRAM_H +#define GRPC_SUPPORT_HISTOGRAM_H + +#include +#include + +#ifdef __cplusplus +extern "C" { +#endif + +typedef struct grpc_histogram grpc_histogram; + +grpc_histogram* grpc_histogram_create(double resolution, + double max_bucket_start); +void grpc_histogram_destroy(grpc_histogram* h); +void grpc_histogram_add(grpc_histogram* h, double x); + +/** The following merges the second histogram into the first. It only works + if they have the same buckets and resolution. Returns 0 on failure, 1 + on success */ +int grpc_histogram_merge(grpc_histogram* dst, const grpc_histogram* src); + +double grpc_histogram_percentile(grpc_histogram* histogram, double percentile); +double grpc_histogram_mean(grpc_histogram* histogram); +double grpc_histogram_stddev(grpc_histogram* histogram); +double grpc_histogram_variance(grpc_histogram* histogram); +double grpc_histogram_maximum(grpc_histogram* histogram); +double grpc_histogram_minimum(grpc_histogram* histogram); +double grpc_histogram_count(grpc_histogram* histogram); +double grpc_histogram_sum(grpc_histogram* histogram); +double grpc_histogram_sum_of_squares(grpc_histogram* histogram); + +const uint32_t* grpc_histogram_get_contents(grpc_histogram* histogram, + size_t* count); +void grpc_histogram_merge_contents(grpc_histogram* histogram, + const uint32_t* data, size_t data_count, + double min_seen, double max_seen, double sum, + double sum_of_squares, double count); + +#ifdef __cplusplus +} +#endif + +#endif /* GRPC_SUPPORT_HISTOGRAM_H */ diff --git a/test/core/util/histogram_test.cc b/test/core/util/histogram_test.cc new file mode 100644 index 00000000000..b96ac7d8419 --- /dev/null +++ b/test/core/util/histogram_test.cc @@ -0,0 +1,163 @@ +/* + * + * Copyright 2015 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +#include "test/core/util/histogram.h" +#include + +#define LOG_TEST(x) gpr_log(GPR_INFO, "%s", x); + +static void test_no_op(void) { + grpc_histogram_destroy(grpc_histogram_create(0.01, 60e9)); +} + +static void expect_percentile(grpc_histogram* h, double percentile, + double min_expect, double max_expect) { + double got = grpc_histogram_percentile(h, percentile); + gpr_log(GPR_INFO, "@%f%%, expect %f <= %f <= %f", percentile, min_expect, got, + max_expect); + GPR_ASSERT(min_expect <= got); + GPR_ASSERT(got <= max_expect); +} + +static void test_simple(void) { + grpc_histogram* h; + + LOG_TEST("test_simple"); + + h = grpc_histogram_create(0.01, 60e9); + grpc_histogram_add(h, 10000); + grpc_histogram_add(h, 10000); + grpc_histogram_add(h, 11000); + grpc_histogram_add(h, 11000); + + expect_percentile(h, 50, 10001, 10999); + GPR_ASSERT(grpc_histogram_mean(h) == 10500); + + grpc_histogram_destroy(h); +} + +static void test_percentile(void) { + grpc_histogram* h; + double last; + double i; + double cur; + + LOG_TEST("test_percentile"); + + h = grpc_histogram_create(0.05, 1e9); + grpc_histogram_add(h, 2.5); + grpc_histogram_add(h, 2.5); + grpc_histogram_add(h, 8); + grpc_histogram_add(h, 4); + + GPR_ASSERT(grpc_histogram_count(h) == 4); + GPR_ASSERT(grpc_histogram_minimum(h) == 2.5); + GPR_ASSERT(grpc_histogram_maximum(h) == 8); + GPR_ASSERT(grpc_histogram_sum(h) == 17); + GPR_ASSERT(grpc_histogram_sum_of_squares(h) == 92.5); + GPR_ASSERT(grpc_histogram_mean(h) == 4.25); + GPR_ASSERT(grpc_histogram_variance(h) == 5.0625); + GPR_ASSERT(grpc_histogram_stddev(h) == 2.25); + + expect_percentile(h, -10, 2.5, 2.5); + expect_percentile(h, 0, 2.5, 2.5); + expect_percentile(h, 12.5, 2.5, 2.5); + expect_percentile(h, 25, 2.5, 2.5); + expect_percentile(h, 37.5, 2.5, 2.8); + expect_percentile(h, 50, 3.0, 3.5); + expect_percentile(h, 62.5, 3.5, 4.5); + expect_percentile(h, 75, 5, 7.9); + expect_percentile(h, 100, 8, 8); + expect_percentile(h, 110, 8, 8); + + /* test monotonicity */ + last = 0.0; + for (i = 0; i < 100.0; i += 0.01) { + cur = grpc_histogram_percentile(h, i); + GPR_ASSERT(cur >= last); + last = cur; + } + + grpc_histogram_destroy(h); +} + +static void test_merge(void) { + grpc_histogram *h1, *h2; + double last; + double i; + double cur; + + LOG_TEST("test_merge"); + + h1 = grpc_histogram_create(0.05, 1e9); + grpc_histogram_add(h1, 2.5); + grpc_histogram_add(h1, 2.5); + grpc_histogram_add(h1, 8); + grpc_histogram_add(h1, 4); + + h2 = grpc_histogram_create(0.01, 1e9); + GPR_ASSERT(grpc_histogram_merge(h1, h2) == 0); + grpc_histogram_destroy(h2); + + h2 = grpc_histogram_create(0.05, 1e10); + GPR_ASSERT(grpc_histogram_merge(h1, h2) == 0); + grpc_histogram_destroy(h2); + + h2 = grpc_histogram_create(0.05, 1e9); + GPR_ASSERT(grpc_histogram_merge(h1, h2) == 1); + GPR_ASSERT(grpc_histogram_count(h1) == 4); + GPR_ASSERT(grpc_histogram_minimum(h1) == 2.5); + GPR_ASSERT(grpc_histogram_maximum(h1) == 8); + GPR_ASSERT(grpc_histogram_sum(h1) == 17); + GPR_ASSERT(grpc_histogram_sum_of_squares(h1) == 92.5); + GPR_ASSERT(grpc_histogram_mean(h1) == 4.25); + GPR_ASSERT(grpc_histogram_variance(h1) == 5.0625); + GPR_ASSERT(grpc_histogram_stddev(h1) == 2.25); + grpc_histogram_destroy(h2); + + h2 = grpc_histogram_create(0.05, 1e9); + grpc_histogram_add(h2, 7.0); + grpc_histogram_add(h2, 17.0); + grpc_histogram_add(h2, 1.0); + GPR_ASSERT(grpc_histogram_merge(h1, h2) == 1); + GPR_ASSERT(grpc_histogram_count(h1) == 7); + GPR_ASSERT(grpc_histogram_minimum(h1) == 1.0); + GPR_ASSERT(grpc_histogram_maximum(h1) == 17.0); + GPR_ASSERT(grpc_histogram_sum(h1) == 42.0); + GPR_ASSERT(grpc_histogram_sum_of_squares(h1) == 431.5); + GPR_ASSERT(grpc_histogram_mean(h1) == 6.0); + + /* test monotonicity */ + last = 0.0; + for (i = 0; i < 100.0; i += 0.01) { + cur = grpc_histogram_percentile(h1, i); + GPR_ASSERT(cur >= last); + last = cur; + } + + grpc_histogram_destroy(h1); + grpc_histogram_destroy(h2); +} + +int main(void) { + test_no_op(); + test_simple(); + test_percentile(); + test_merge(); + return 0; +} diff --git a/test/cpp/qps/BUILD b/test/cpp/qps/BUILD index 0d91d52f221..f1abb19e64d 100644 --- a/test/cpp/qps/BUILD +++ b/test/cpp/qps/BUILD @@ -106,7 +106,7 @@ grpc_cc_library( "histogram.h", "stats.h", ], - deps = ["//:gpr"], + deps = ["//test/core/util:grpc_test_util"], ) grpc_cc_test( diff --git a/test/cpp/qps/histogram.h b/test/cpp/qps/histogram.h index e31d5d78a8e..ba72b5b3320 100644 --- a/test/cpp/qps/histogram.h +++ b/test/cpp/qps/histogram.h @@ -19,8 +19,8 @@ #ifndef TEST_QPS_HISTOGRAM_H #define TEST_QPS_HISTOGRAM_H -#include #include "src/proto/grpc/testing/stats.pb.h" +#include "test/core/util/histogram.h" namespace grpc { namespace testing { @@ -29,36 +29,36 @@ class Histogram { public: // TODO: look into making histogram params not hardcoded for C++ Histogram() - : impl_(gpr_histogram_create(default_resolution(), - default_max_possible())) {} + : impl_(grpc_histogram_create(default_resolution(), + default_max_possible())) {} ~Histogram() { - if (impl_) gpr_histogram_destroy(impl_); + if (impl_) grpc_histogram_destroy(impl_); } Histogram(Histogram&& other) : impl_(other.impl_) { other.impl_ = nullptr; } - void Merge(const Histogram& h) { gpr_histogram_merge(impl_, h.impl_); } - void Add(double value) { gpr_histogram_add(impl_, value); } + void Merge(const Histogram& h) { grpc_histogram_merge(impl_, h.impl_); } + void Add(double value) { grpc_histogram_add(impl_, value); } double Percentile(double pctile) const { - return gpr_histogram_percentile(impl_, pctile); + return grpc_histogram_percentile(impl_, pctile); } - double Count() const { return gpr_histogram_count(impl_); } + double Count() const { return grpc_histogram_count(impl_); } void Swap(Histogram* other) { std::swap(impl_, other->impl_); } void FillProto(HistogramData* p) { size_t n; - const auto* data = gpr_histogram_get_contents(impl_, &n); + const auto* data = grpc_histogram_get_contents(impl_, &n); for (size_t i = 0; i < n; i++) { p->add_bucket(data[i]); } - p->set_min_seen(gpr_histogram_minimum(impl_)); - p->set_max_seen(gpr_histogram_maximum(impl_)); - p->set_sum(gpr_histogram_sum(impl_)); - p->set_sum_of_squares(gpr_histogram_sum_of_squares(impl_)); - p->set_count(gpr_histogram_count(impl_)); + p->set_min_seen(grpc_histogram_minimum(impl_)); + p->set_max_seen(grpc_histogram_maximum(impl_)); + p->set_sum(grpc_histogram_sum(impl_)); + p->set_sum_of_squares(grpc_histogram_sum_of_squares(impl_)); + p->set_count(grpc_histogram_count(impl_)); } void MergeProto(const HistogramData& p) { - gpr_histogram_merge_contents(impl_, &*p.bucket().begin(), p.bucket_size(), - p.min_seen(), p.max_seen(), p.sum(), - p.sum_of_squares(), p.count()); + grpc_histogram_merge_contents(impl_, &*p.bucket().begin(), p.bucket_size(), + p.min_seen(), p.max_seen(), p.sum(), + p.sum_of_squares(), p.count()); } static double default_resolution() { return 0.01; } @@ -68,7 +68,7 @@ class Histogram { Histogram(const Histogram&); Histogram& operator=(const Histogram&); - gpr_histogram* impl_; + grpc_histogram* impl_; }; } // namespace testing } // namespace grpc diff --git a/test/cpp/qps/qps_interarrival_test.cc b/test/cpp/qps/qps_interarrival_test.cc index 461bf624cec..625b7db4269 100644 --- a/test/cpp/qps/qps_interarrival_test.cc +++ b/test/cpp/qps/qps_interarrival_test.cc @@ -20,7 +20,7 @@ #include // Use the C histogram rather than C++ to avoid depending on proto -#include +#include "test/core/util/histogram.h" #include "test/cpp/qps/interarrival.h" #include "test/cpp/util/test_config.h" @@ -31,21 +31,21 @@ using grpc::testing::RandomDistInterface; static void RunTest(RandomDistInterface&& r, int threads, std::string title) { InterarrivalTimer timer; timer.init(r, threads); - gpr_histogram* h(gpr_histogram_create(0.01, 60e9)); + grpc_histogram* h(grpc_histogram_create(0.01, 60e9)); for (int i = 0; i < 10000000; i++) { for (int j = 0; j < threads; j++) { - gpr_histogram_add(h, timer.next(j)); + grpc_histogram_add(h, timer.next(j)); } } std::cout << title << " Distribution" << std::endl; std::cout << "Value, Percentile" << std::endl; for (double pct = 0.0; pct < 100.0; pct += 1.0) { - std::cout << gpr_histogram_percentile(h, pct) << "," << pct << std::endl; + std::cout << grpc_histogram_percentile(h, pct) << "," << pct << std::endl; } - gpr_histogram_destroy(h); + grpc_histogram_destroy(h); } using grpc::testing::ExpDist; diff --git a/test/cpp/qps/qps_worker.cc b/test/cpp/qps/qps_worker.cc index c288b03ec51..4c9ab0ea43e 100644 --- a/test/cpp/qps/qps_worker.cc +++ b/test/cpp/qps/qps_worker.cc @@ -32,12 +32,12 @@ #include #include #include -#include #include #include #include "src/proto/grpc/testing/services.pb.h" #include "test/core/util/grpc_profiler.h" +#include "test/core/util/histogram.h" #include "test/cpp/qps/client.h" #include "test/cpp/qps/server.h" #include "test/cpp/util/create_test_channel.h" diff --git a/tools/doxygen/Doxyfile.c++ b/tools/doxygen/Doxyfile.c++ index bd641c25422..e62278cb9f6 100644 --- a/tools/doxygen/Doxyfile.c++ +++ b/tools/doxygen/Doxyfile.c++ @@ -913,7 +913,6 @@ include/grpc/support/atm_windows.h \ include/grpc/support/avl.h \ include/grpc/support/cmdline.h \ include/grpc/support/cpu.h \ -include/grpc/support/histogram.h \ include/grpc/support/host_port.h \ include/grpc/support/log.h \ include/grpc/support/log_windows.h \ diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index 09f668d847e..d09b325c97a 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -914,7 +914,6 @@ include/grpc/support/atm_windows.h \ include/grpc/support/avl.h \ include/grpc/support/cmdline.h \ include/grpc/support/cpu.h \ -include/grpc/support/histogram.h \ include/grpc/support/host_port.h \ include/grpc/support/log.h \ include/grpc/support/log_windows.h \ diff --git a/tools/doxygen/Doxyfile.core b/tools/doxygen/Doxyfile.core index 97bb43a4238..6ce90417475 100644 --- a/tools/doxygen/Doxyfile.core +++ b/tools/doxygen/Doxyfile.core @@ -853,7 +853,6 @@ include/grpc/support/atm_windows.h \ include/grpc/support/avl.h \ include/grpc/support/cmdline.h \ include/grpc/support/cpu.h \ -include/grpc/support/histogram.h \ include/grpc/support/host_port.h \ include/grpc/support/log.h \ include/grpc/support/log_windows.h \ diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index 28b7d396e00..1aff0075a6a 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -853,7 +853,6 @@ include/grpc/support/atm_windows.h \ include/grpc/support/avl.h \ include/grpc/support/cmdline.h \ include/grpc/support/cpu.h \ -include/grpc/support/histogram.h \ include/grpc/support/host_port.h \ include/grpc/support/log.h \ include/grpc/support/log_windows.h \ @@ -1294,7 +1293,6 @@ src/core/lib/support/env_posix.cc \ src/core/lib/support/env_windows.cc \ src/core/lib/support/fork.cc \ src/core/lib/support/fork.h \ -src/core/lib/support/histogram.cc \ src/core/lib/support/host_port.cc \ src/core/lib/support/log.cc \ src/core/lib/support/log_android.cc \ diff --git a/tools/run_tests/generated/sources_and_headers.json b/tools/run_tests/generated/sources_and_headers.json index c868acf8d30..a0ea4238e1e 100644 --- a/tools/run_tests/generated/sources_and_headers.json +++ b/tools/run_tests/generated/sources_and_headers.json @@ -703,21 +703,6 @@ "third_party": false, "type": "target" }, - { - "deps": [ - "gpr", - "gpr_test_util" - ], - "headers": [], - "is_filegroup": false, - "language": "c", - "name": "gpr_histogram_test", - "src": [ - "test/core/support/histogram_test.cc" - ], - "third_party": false, - "type": "target" - }, { "deps": [ "gpr", @@ -1242,6 +1227,21 @@ "third_party": false, "type": "target" }, + { + "deps": [ + "gpr", + "grpc_test_util" + ], + "headers": [], + "is_filegroup": false, + "language": "c", + "name": "histogram_test", + "src": [ + "test/core/util/histogram_test.cc" + ], + "third_party": false, + "type": "target" + }, { "deps": [ "gpr", @@ -7766,7 +7766,6 @@ "src/core/lib/support/env_posix.cc", "src/core/lib/support/env_windows.cc", "src/core/lib/support/fork.cc", - "src/core/lib/support/histogram.cc", "src/core/lib/support/host_port.cc", "src/core/lib/support/log.cc", "src/core/lib/support/log_android.cc", @@ -7813,7 +7812,6 @@ "include/grpc/support/avl.h", "include/grpc/support/cmdline.h", "include/grpc/support/cpu.h", - "include/grpc/support/histogram.h", "include/grpc/support/host_port.h", "include/grpc/support/log.h", "include/grpc/support/log_windows.h", @@ -7863,7 +7861,6 @@ "include/grpc/support/avl.h", "include/grpc/support/cmdline.h", "include/grpc/support/cpu.h", - "include/grpc/support/histogram.h", "include/grpc/support/host_port.h", "include/grpc/support/log.h", "include/grpc/support/log_windows.h", @@ -8928,6 +8925,7 @@ "test/core/iomgr/endpoint_tests.h", "test/core/util/debugger_macros.h", "test/core/util/grpc_profiler.h", + "test/core/util/histogram.h", "test/core/util/memory_counters.h", "test/core/util/mock_endpoint.h", "test/core/util/parse_hexstring.h", @@ -8956,6 +8954,8 @@ "test/core/util/debugger_macros.h", "test/core/util/grpc_profiler.cc", "test/core/util/grpc_profiler.h", + "test/core/util/histogram.cc", + "test/core/util/histogram.h", "test/core/util/memory_counters.cc", "test/core/util/memory_counters.h", "test/core/util/mock_endpoint.cc", diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index 929ed159187..ce698cb709e 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -839,30 +839,6 @@ ], "uses_polling": false }, - { - "args": [], - "benchmark": false, - "ci_platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "gtest": false, - "language": "c", - "name": "gpr_histogram_test", - "platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "uses_polling": false - }, { "args": [], "benchmark": false, @@ -1523,6 +1499,30 @@ ], "uses_polling": true }, + { + "args": [], + "benchmark": false, + "ci_platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "gtest": false, + "language": "c", + "name": "histogram_test", + "platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "uses_polling": false + }, { "args": [], "benchmark": false, From b9cb3379d541a5d7c43f3b4e999d2b4a87bca695 Mon Sep 17 00:00:00 2001 From: Adele Zhou Date: Fri, 17 Nov 2017 10:40:39 -0800 Subject: [PATCH 14/39] Add bazel_toolchains --- WORKSPACE | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/WORKSPACE b/WORKSPACE index cead0bc877b..43cf87e5a02 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -115,3 +115,13 @@ http_archive( strip_prefix = "abseil-cpp-cc4bed2d74f7c8717e31f9579214ab52a9c9c610", url = "https://github.com/abseil/abseil-cpp/archive/cc4bed2d74f7c8717e31f9579214ab52a9c9c610.tar.gz", ) + +http_archive( + name = "bazel_toolchains", + urls = [ + "http://mirror.bazel.build/github.com/bazelbuild/bazel-toolchains/archive/af4681c3d19f063f090222ec3d04108c4e0ca255.tar.gz", + "https://github.com/bazelbuild/bazel-toolchains/archive/af4681c3d19f063f090222ec3d04108c4e0ca255.tar.gz", + ], + strip_prefix = "bazel-toolchains-af4681c3d19f063f090222ec3d04108c4e0ca255", + sha256 = "d58bb2d6c8603f600d522b6104d6192a65339aa26cbba9f11ff5c4b36dedb928", +) From 6c38fbe4be96e6071f23e86802f618af790d0459 Mon Sep 17 00:00:00 2001 From: Adele Zhou Date: Fri, 17 Nov 2017 15:45:11 -0800 Subject: [PATCH 15/39] Experimental build script to use Foundry. --- .../linux/grpc_bazel_on_foundry.sh | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 tools/internal_ci/linux/grpc_bazel_on_foundry.sh diff --git a/tools/internal_ci/linux/grpc_bazel_on_foundry.sh b/tools/internal_ci/linux/grpc_bazel_on_foundry.sh new file mode 100644 index 00000000000..fc8727f5a0c --- /dev/null +++ b/tools/internal_ci/linux/grpc_bazel_on_foundry.sh @@ -0,0 +1,43 @@ +#!/usr/bin/env bash +# Copyright 2017 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +set -ex -o igncr || set -ex + +which bazel +chmod +x "${KOKORO_GFILE_DIR}/bazel_wrapper.py" + +# change to grpc repo root +cd $(dirname $0)/../../.. + +source tools/internal_ci/helper_scripts/prepare_build_linux_rc + +"${KOKORO_GFILE_DIR}/bazel_wrapper.py" \ + --host_jvm_args=-Dbazel.DigestFunction=SHA1 \ + test --jobs="50" \ + --test_timeout="300,450,1200,3600" \ + --test_output=errors \ + --verbose_failures=true \ + --keep_going \ + --remote_accept_cached=true \ + --spawn_strategy=remote \ + --remote_local_fallback=false \ + --remote_timeout=3600 \ + --strategy=Javac=remote \ + --strategy=Closure=remote \ + --genrule_strategy=remote \ + --experimental_strict_action_env=true \ + --experimental_remote_platform_override='properties:{ name:"container-image" value:"docker://gcr.io/asci-toolchain/nosla-debian8-clang-fl@sha256:b80da64e2a6f75af122bbb70021ebaab98b073f144ab04653c0de49bd943d8e9" }' \ + --crosstool_top=@bazel_toolchains//configs/debian8_clang/0.1.0/bazel_0.6.0:toolchain \ + -- //test/... From 020603d1cfd42e5c3fcecea028163ec372ed492d Mon Sep 17 00:00:00 2001 From: Adele Zhou Date: Tue, 21 Nov 2017 11:21:11 -0800 Subject: [PATCH 16/39] Use canary bazel --- tools/internal_ci/linux/grpc_bazel_on_foundry.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tools/internal_ci/linux/grpc_bazel_on_foundry.sh b/tools/internal_ci/linux/grpc_bazel_on_foundry.sh index fc8727f5a0c..58055bb37a2 100644 --- a/tools/internal_ci/linux/grpc_bazel_on_foundry.sh +++ b/tools/internal_ci/linux/grpc_bazel_on_foundry.sh @@ -13,8 +13,13 @@ # See the License for the specific language governing permissions and # limitations under the License. -set -ex -o igncr || set -ex +set -ex +mkdir -p /tmpfs/tmp/bazel-canary +ln -f "${KOKORO_GFILE_DIR}/bazel-canary" /tmpfs/tmp/bazel-canary/bazel +chmod 755 "${KOKORO_GFILE_DIR}/bazel-canary" +export PATH="/tmpfs/tmp/bazel-canary:${PATH}" +# This should show /tmpfs/tmp/bazel-canary/bazel which bazel chmod +x "${KOKORO_GFILE_DIR}/bazel_wrapper.py" From 0384127aa8bb4a0f244826c434b623299157e891 Mon Sep 17 00:00:00 2001 From: Adele Zhou Date: Tue, 21 Nov 2017 15:24:37 -0800 Subject: [PATCH 17/39] Copy key data --- WORKSPACE | 2 +- tools/internal_ci/linux/grpc_bazel_on_foundry.sh | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 43cf87e5a02..bf09aa7cc93 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -119,7 +119,7 @@ http_archive( http_archive( name = "bazel_toolchains", urls = [ - "http://mirror.bazel.build/github.com/bazelbuild/bazel-toolchains/archive/af4681c3d19f063f090222ec3d04108c4e0ca255.tar.gz", + "https://mirror.bazel.build/github.com/bazelbuild/bazel-toolchains/archive/af4681c3d19f063f090222ec3d04108c4e0ca255.tar.gz", "https://github.com/bazelbuild/bazel-toolchains/archive/af4681c3d19f063f090222ec3d04108c4e0ca255.tar.gz", ], strip_prefix = "bazel-toolchains-af4681c3d19f063f090222ec3d04108c4e0ca255", diff --git a/tools/internal_ci/linux/grpc_bazel_on_foundry.sh b/tools/internal_ci/linux/grpc_bazel_on_foundry.sh index 58055bb37a2..3ad0fa6c6bf 100644 --- a/tools/internal_ci/linux/grpc_bazel_on_foundry.sh +++ b/tools/internal_ci/linux/grpc_bazel_on_foundry.sh @@ -15,6 +15,9 @@ set -ex +mkdir -p ${KOKORO_KEYSTORE_DIR} +cp ${KOKORO_GFILE_DIR}/GrpcTesting-d0eeee2db331.json ${KOKORO_KEYSTORE_DIR}/4321_grpc-testing-service + mkdir -p /tmpfs/tmp/bazel-canary ln -f "${KOKORO_GFILE_DIR}/bazel-canary" /tmpfs/tmp/bazel-canary/bazel chmod 755 "${KOKORO_GFILE_DIR}/bazel-canary" @@ -44,5 +47,5 @@ source tools/internal_ci/helper_scripts/prepare_build_linux_rc --genrule_strategy=remote \ --experimental_strict_action_env=true \ --experimental_remote_platform_override='properties:{ name:"container-image" value:"docker://gcr.io/asci-toolchain/nosla-debian8-clang-fl@sha256:b80da64e2a6f75af122bbb70021ebaab98b073f144ab04653c0de49bd943d8e9" }' \ - --crosstool_top=@bazel_toolchains//configs/debian8_clang/0.1.0/bazel_0.6.0:toolchain \ - -- //test/... + --crosstool_top=@bazel_toolchains//configs/debian8_clang/0.2.0/bazel_0.7.0:toolchain \ + -- //test/core/iomgr/... From 379105d4f0a07cec0a708ce3b0ad9ed686d05457 Mon Sep 17 00:00:00 2001 From: Adele Zhou Date: Tue, 28 Nov 2017 16:06:09 -0800 Subject: [PATCH 18/39] Update toolchain version. --- tools/internal_ci/linux/grpc_bazel_on_foundry.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/internal_ci/linux/grpc_bazel_on_foundry.sh b/tools/internal_ci/linux/grpc_bazel_on_foundry.sh index 3ad0fa6c6bf..c45d43a386b 100644 --- a/tools/internal_ci/linux/grpc_bazel_on_foundry.sh +++ b/tools/internal_ci/linux/grpc_bazel_on_foundry.sh @@ -46,6 +46,6 @@ source tools/internal_ci/helper_scripts/prepare_build_linux_rc --strategy=Closure=remote \ --genrule_strategy=remote \ --experimental_strict_action_env=true \ - --experimental_remote_platform_override='properties:{ name:"container-image" value:"docker://gcr.io/asci-toolchain/nosla-debian8-clang-fl@sha256:b80da64e2a6f75af122bbb70021ebaab98b073f144ab04653c0de49bd943d8e9" }' \ + --experimental_remote_platform_override='properties:{name:"container-image" value:"docker://gcr.io/asci-toolchain/nosla-debian8-clang-fl@sha256:aa20628a902f06a11a015caa94b0432eb60690de2d2525bd046b9eea046f5d8a" }' \ --crosstool_top=@bazel_toolchains//configs/debian8_clang/0.2.0/bazel_0.7.0:toolchain \ - -- //test/core/iomgr/... + -- //test/... From c8bf2da1e0f36fe3a6f271140153fad8e9df8dbd Mon Sep 17 00:00:00 2001 From: Adele Zhou Date: Wed, 29 Nov 2017 09:59:34 -0800 Subject: [PATCH 19/39] Enabling hermetic testing for running tests on Foundry. --- CMakeLists.txt | 2 + Makefile | 2 + bazel/grpc_build_system.bzl | 4 +- build.yaml | 1 + grpc.gyp | 2 + test/core/util/BUILD | 4 ++ test/core/util/port_hermetic.cc | 41 +++++++++++++++++++ test/core/util/test_config.h | 2 +- third_party/zlib.BUILD | 2 +- .../generated/sources_and_headers.json | 3 +- 10 files changed, 58 insertions(+), 5 deletions(-) create mode 100644 test/core/util/port_hermetic.cc diff --git a/CMakeLists.txt b/CMakeLists.txt index 760e4fa5852..3bb627aa7de 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1622,6 +1622,7 @@ add_library(grpc_test_util test/core/util/parse_hexstring.cc test/core/util/passthru_endpoint.cc test/core/util/port.cc + test/core/util/port_hermetic.cc test/core/util/port_server_client.cc test/core/util/slice_splitter.cc test/core/util/tracer_util.cc @@ -1890,6 +1891,7 @@ add_library(grpc_test_util_unsecure test/core/util/parse_hexstring.cc test/core/util/passthru_endpoint.cc test/core/util/port.cc + test/core/util/port_hermetic.cc test/core/util/port_server_client.cc test/core/util/slice_splitter.cc test/core/util/tracer_util.cc diff --git a/Makefile b/Makefile index 3ce30059241..f5106cb2654 100644 --- a/Makefile +++ b/Makefile @@ -3628,6 +3628,7 @@ LIBGRPC_TEST_UTIL_SRC = \ test/core/util/parse_hexstring.cc \ test/core/util/passthru_endpoint.cc \ test/core/util/port.cc \ + test/core/util/port_hermetic.cc \ test/core/util/port_server_client.cc \ test/core/util/slice_splitter.cc \ test/core/util/tracer_util.cc \ @@ -3887,6 +3888,7 @@ LIBGRPC_TEST_UTIL_UNSECURE_SRC = \ test/core/util/parse_hexstring.cc \ test/core/util/passthru_endpoint.cc \ test/core/util/port.cc \ + test/core/util/port_hermetic.cc \ test/core/util/port_server_client.cc \ test/core/util/slice_splitter.cc \ test/core/util/tracer_util.cc \ diff --git a/bazel/grpc_build_system.bzl b/bazel/grpc_build_system.bzl index b35ca73745f..591fdb5936a 100644 --- a/bazel/grpc_build_system.bzl +++ b/bazel/grpc_build_system.bzl @@ -26,7 +26,7 @@ def grpc_cc_library(name, srcs = [], public_hdrs = [], hdrs = [], external_deps = [], deps = [], standalone = False, language = "C++", testonly = False, visibility = None, - alwayslink = 0): + alwayslink = 0, defines = []): copts = [] if language.upper() == "C": copts = ["-std=c99"] @@ -35,7 +35,7 @@ def grpc_cc_library(name, srcs = [], public_hdrs = [], hdrs = [], srcs = srcs, defines = select({ "//:grpc_no_ares": ["GRPC_ARES=0"], - "//conditions:default": [], + "//conditions:default": defines, }), hdrs = hdrs + public_hdrs, deps = deps + ["//external:" + dep for dep in external_deps], diff --git a/build.yaml b/build.yaml index 0908f7efd23..0f6ac5631c5 100644 --- a/build.yaml +++ b/build.yaml @@ -736,6 +736,7 @@ filegroups: - test/core/util/parse_hexstring.cc - test/core/util/passthru_endpoint.cc - test/core/util/port.cc + - test/core/util/port_hermetic.cc - test/core/util/port_server_client.cc - test/core/util/slice_splitter.cc - test/core/util/tracer_util.cc diff --git a/grpc.gyp b/grpc.gyp index f2033b47b0b..7e6a06c8761 100644 --- a/grpc.gyp +++ b/grpc.gyp @@ -511,6 +511,7 @@ 'test/core/util/parse_hexstring.cc', 'test/core/util/passthru_endpoint.cc', 'test/core/util/port.cc', + 'test/core/util/port_hermetic.cc', 'test/core/util/port_server_client.cc', 'test/core/util/slice_splitter.cc', 'test/core/util/tracer_util.cc', @@ -721,6 +722,7 @@ 'test/core/util/parse_hexstring.cc', 'test/core/util/passthru_endpoint.cc', 'test/core/util/port.cc', + 'test/core/util/port_hermetic.cc', 'test/core/util/port_server_client.cc', 'test/core/util/slice_splitter.cc', 'test/core/util/tracer_util.cc', diff --git a/test/core/util/BUILD b/test/core/util/BUILD index 6443553466f..9884e04627a 100644 --- a/test/core/util/BUILD +++ b/test/core/util/BUILD @@ -53,6 +53,7 @@ grpc_cc_library( "parse_hexstring.cc", "passthru_endpoint.cc", "port.cc", + "port_hermetic.cc", "port_server_client.cc", "reconnect_server.cc", "slice_splitter.cc", @@ -73,6 +74,9 @@ grpc_cc_library( "tracer_util.h", "trickle_endpoint.h", ], + defines = [ + "GRPC_HERMETIC_TESTS=1", + ], language = "C++", deps = [ ":gpr_test_util", diff --git a/test/core/util/port_hermetic.cc b/test/core/util/port_hermetic.cc new file mode 100644 index 00000000000..36508167637 --- /dev/null +++ b/test/core/util/port_hermetic.cc @@ -0,0 +1,41 @@ +/* + * + * Copyright 2017 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +#include "src/core/lib/iomgr/port.h" +#include "test/core/util/test_config.h" +#if defined(GRPC_HERMETIC_TESTS) + +#include "test/core/util/port.h" + +#define LOWER_PORT 49152 +static int s_allocated_port = LOWER_PORT; + +int grpc_pick_unused_port_or_die(void) { + int allocated_port = s_allocated_port++; + if (s_allocated_port == 65536) { + s_allocated_port = LOWER_PORT; + } + + return allocated_port; +} + +void grpc_recycle_unused_port(int port) { + (void) port; +} + +#endif /* GRPC_HERMETIC_TESTS */ diff --git a/test/core/util/test_config.h b/test/core/util/test_config.h index 775ffac9497..ee60441653a 100644 --- a/test/core/util/test_config.h +++ b/test/core/util/test_config.h @@ -33,7 +33,7 @@ gpr_timespec grpc_timeout_seconds_to_deadline(int64_t time_s); /* Converts a given timeout (in milliseconds) to a deadline. */ gpr_timespec grpc_timeout_milliseconds_to_deadline(int64_t time_ms); -#ifndef GRPC_TEST_CUSTOM_PICK_PORT +#if !defined(GRPC_TEST_CUSTOM_PICK_PORT) && !defined(GRPC_HERMETIC_TESTS) #define GRPC_TEST_PICK_PORT #endif diff --git a/third_party/zlib.BUILD b/third_party/zlib.BUILD index 7879a81c680..a71c85fa98d 100644 --- a/third_party/zlib.BUILD +++ b/third_party/zlib.BUILD @@ -27,7 +27,7 @@ cc_library( "zutil.h", ], includes = [ - "include", + ".", ], linkstatic = 1, visibility = [ diff --git a/tools/run_tests/generated/sources_and_headers.json b/tools/run_tests/generated/sources_and_headers.json index c868acf8d30..c957638181f 100644 --- a/tools/run_tests/generated/sources_and_headers.json +++ b/tools/run_tests/generated/sources_and_headers.json @@ -8932,7 +8932,8 @@ "test/core/util/mock_endpoint.h", "test/core/util/parse_hexstring.h", "test/core/util/passthru_endpoint.h", - "test/core/util/port.h", + "test/core/util/port.h", + "test/core/util/port_hermetic.cc", "test/core/util/port_server_client.h", "test/core/util/slice_splitter.h", "test/core/util/tracer_util.h", From e5df91f477a27ce71a39bca928677b2f08649615 Mon Sep 17 00:00:00 2001 From: Adele Zhou Date: Wed, 29 Nov 2017 14:37:18 -0800 Subject: [PATCH 20/39] Use select to decide the test is hermetic or not --- BUILD | 5 +++++ bazel/grpc_build_system.bzl | 10 +++++----- test/core/util/BUILD | 3 --- test/core/util/port_hermetic.cc | 4 ++++ tools/internal_ci/linux/grpc_bazel_on_foundry.sh | 1 + 5 files changed, 15 insertions(+), 8 deletions(-) diff --git a/BUILD b/BUILD index 30b410a2bd1..d997dc253c4 100644 --- a/BUILD +++ b/BUILD @@ -38,6 +38,11 @@ config_setting( values = {"define": "grpc_no_ares=true"}, ) +config_setting( + name = "remote_execution", + values = {"define": "GRPC_HERMETIC_TESTS=1"}, +) + # This should be updated along with build.yaml g_stands_for = "generous" diff --git a/bazel/grpc_build_system.bzl b/bazel/grpc_build_system.bzl index 591fdb5936a..a6da3772481 100644 --- a/bazel/grpc_build_system.bzl +++ b/bazel/grpc_build_system.bzl @@ -26,17 +26,17 @@ def grpc_cc_library(name, srcs = [], public_hdrs = [], hdrs = [], external_deps = [], deps = [], standalone = False, language = "C++", testonly = False, visibility = None, - alwayslink = 0, defines = []): + alwayslink = 0): copts = [] if language.upper() == "C": copts = ["-std=c99"] native.cc_library( name = name, srcs = srcs, - defines = select({ - "//:grpc_no_ares": ["GRPC_ARES=0"], - "//conditions:default": defines, - }), + defines = select({"//:grpc_no_ares": ["GRPC_ARES=0"], + "//conditions:default": [],}) + + select({"//:remote_execution": ["GRPC_HERMETIC_TESTS=1"], + "//conditions:default": [],}), hdrs = hdrs + public_hdrs, deps = deps + ["//external:" + dep for dep in external_deps], copts = copts, diff --git a/test/core/util/BUILD b/test/core/util/BUILD index 9884e04627a..f92c0ff5489 100644 --- a/test/core/util/BUILD +++ b/test/core/util/BUILD @@ -74,9 +74,6 @@ grpc_cc_library( "tracer_util.h", "trickle_endpoint.h", ], - defines = [ - "GRPC_HERMETIC_TESTS=1", - ], language = "C++", deps = [ ":gpr_test_util", diff --git a/test/core/util/port_hermetic.cc b/test/core/util/port_hermetic.cc index 36508167637..b4d097f650d 100644 --- a/test/core/util/port_hermetic.cc +++ b/test/core/util/port_hermetic.cc @@ -16,6 +16,10 @@ * */ +/* When running tests hermeticly, i.e. running on remote machines, + * the framework takes a round-robin pick of a port within certain range. + * There is no need to recycle ports. + */ #include "src/core/lib/iomgr/port.h" #include "test/core/util/test_config.h" #if defined(GRPC_HERMETIC_TESTS) diff --git a/tools/internal_ci/linux/grpc_bazel_on_foundry.sh b/tools/internal_ci/linux/grpc_bazel_on_foundry.sh index c45d43a386b..dc6df47ac81 100644 --- a/tools/internal_ci/linux/grpc_bazel_on_foundry.sh +++ b/tools/internal_ci/linux/grpc_bazel_on_foundry.sh @@ -48,4 +48,5 @@ source tools/internal_ci/helper_scripts/prepare_build_linux_rc --experimental_strict_action_env=true \ --experimental_remote_platform_override='properties:{name:"container-image" value:"docker://gcr.io/asci-toolchain/nosla-debian8-clang-fl@sha256:aa20628a902f06a11a015caa94b0432eb60690de2d2525bd046b9eea046f5d8a" }' \ --crosstool_top=@bazel_toolchains//configs/debian8_clang/0.2.0/bazel_0.7.0:toolchain \ + --define GRPC_HERMETIC_TESTS=1 \ -- //test/... From c62a83629994140ab8558b001b2c60511c645e2e Mon Sep 17 00:00:00 2001 From: Adele Zhou Date: Thu, 30 Nov 2017 17:36:15 -0800 Subject: [PATCH 21/39] remove redundant python versions. --- tools/interop_matrix/client_matrix.py | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/tools/interop_matrix/client_matrix.py b/tools/interop_matrix/client_matrix.py index ee24ae7b28b..c9a4996029a 100644 --- a/tools/interop_matrix/client_matrix.py +++ b/tools/interop_matrix/client_matrix.py @@ -71,23 +71,6 @@ LANG_RELEASE_MATRIX = { 'v1.7.0', 'v1.8.0', ], - 'python': [ - 'v1.0.x', - 'v1.1.4', - 'v1.2.5', - 'v1.3.9', - 'v1.4.2', - 'v1.6.6', - ], - 'python': [ - 'v1.0.x', - 'v1.1.4', - 'v1.2.5', - 'v1.3.9', - 'v1.4.2', - 'v1.6.6', - 'v1.7.2', - ], 'python': [ 'v1.0.x', 'v1.1.4', From 5b7cdefb681977357573d9eac866520610d19fa5 Mon Sep 17 00:00:00 2001 From: Adele Zhou Date: Wed, 29 Nov 2017 16:25:17 -0800 Subject: [PATCH 22/39] Rename port_hermetic to port_isolated_runtime_environment --- BUILD | 2 +- CMakeLists.txt | 4 ++-- Makefile | 4 ++-- bazel/grpc_build_system.bzl | 2 +- build.yaml | 2 +- grpc.gyp | 4 ++-- test/core/util/BUILD | 2 +- ...etic.cc => port_isolated_runtime_environment.cc} | 13 +++++-------- test/core/util/test_config.h | 2 +- tools/internal_ci/linux/grpc_bazel_on_foundry.sh | 5 ++++- tools/run_tests/generated/sources_and_headers.json | 4 ++-- 11 files changed, 22 insertions(+), 22 deletions(-) rename test/core/util/{port_hermetic.cc => port_isolated_runtime_environment.cc} (76%) diff --git a/BUILD b/BUILD index d997dc253c4..2dd8db06994 100644 --- a/BUILD +++ b/BUILD @@ -40,7 +40,7 @@ config_setting( config_setting( name = "remote_execution", - values = {"define": "GRPC_HERMETIC_TESTS=1"}, + values = {"define": "GRPC_PORT_ISOLATED_RUNTIME=1"}, ) # This should be updated along with build.yaml diff --git a/CMakeLists.txt b/CMakeLists.txt index 3bb627aa7de..975c2975050 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1622,7 +1622,7 @@ add_library(grpc_test_util test/core/util/parse_hexstring.cc test/core/util/passthru_endpoint.cc test/core/util/port.cc - test/core/util/port_hermetic.cc + test/core/util/port_isolated_runtime_environment.cc test/core/util/port_server_client.cc test/core/util/slice_splitter.cc test/core/util/tracer_util.cc @@ -1891,7 +1891,7 @@ add_library(grpc_test_util_unsecure test/core/util/parse_hexstring.cc test/core/util/passthru_endpoint.cc test/core/util/port.cc - test/core/util/port_hermetic.cc + test/core/util/port_isolated_runtime_environment.cc test/core/util/port_server_client.cc test/core/util/slice_splitter.cc test/core/util/tracer_util.cc diff --git a/Makefile b/Makefile index f5106cb2654..6b8116dda91 100644 --- a/Makefile +++ b/Makefile @@ -3628,7 +3628,7 @@ LIBGRPC_TEST_UTIL_SRC = \ test/core/util/parse_hexstring.cc \ test/core/util/passthru_endpoint.cc \ test/core/util/port.cc \ - test/core/util/port_hermetic.cc \ + test/core/util/port_isolated_runtime_environment.cc \ test/core/util/port_server_client.cc \ test/core/util/slice_splitter.cc \ test/core/util/tracer_util.cc \ @@ -3888,7 +3888,7 @@ LIBGRPC_TEST_UTIL_UNSECURE_SRC = \ test/core/util/parse_hexstring.cc \ test/core/util/passthru_endpoint.cc \ test/core/util/port.cc \ - test/core/util/port_hermetic.cc \ + test/core/util/port_isolated_runtime_environment.cc \ test/core/util/port_server_client.cc \ test/core/util/slice_splitter.cc \ test/core/util/tracer_util.cc \ diff --git a/bazel/grpc_build_system.bzl b/bazel/grpc_build_system.bzl index a6da3772481..2ee8c297da1 100644 --- a/bazel/grpc_build_system.bzl +++ b/bazel/grpc_build_system.bzl @@ -35,7 +35,7 @@ def grpc_cc_library(name, srcs = [], public_hdrs = [], hdrs = [], srcs = srcs, defines = select({"//:grpc_no_ares": ["GRPC_ARES=0"], "//conditions:default": [],}) + - select({"//:remote_execution": ["GRPC_HERMETIC_TESTS=1"], + select({"//:remote_execution": ["GRPC_PORT_ISOLATED_RUNTIME=1"], "//conditions:default": [],}), hdrs = hdrs + public_hdrs, deps = deps + ["//external:" + dep for dep in external_deps], diff --git a/build.yaml b/build.yaml index 0f6ac5631c5..df2975fcc20 100644 --- a/build.yaml +++ b/build.yaml @@ -736,7 +736,7 @@ filegroups: - test/core/util/parse_hexstring.cc - test/core/util/passthru_endpoint.cc - test/core/util/port.cc - - test/core/util/port_hermetic.cc + - test/core/util/port_isolated_runtime_environment.cc - test/core/util/port_server_client.cc - test/core/util/slice_splitter.cc - test/core/util/tracer_util.cc diff --git a/grpc.gyp b/grpc.gyp index 7e6a06c8761..b07696d3168 100644 --- a/grpc.gyp +++ b/grpc.gyp @@ -511,7 +511,7 @@ 'test/core/util/parse_hexstring.cc', 'test/core/util/passthru_endpoint.cc', 'test/core/util/port.cc', - 'test/core/util/port_hermetic.cc', + 'test/core/util/port_isolated_runtime_environment.cc', 'test/core/util/port_server_client.cc', 'test/core/util/slice_splitter.cc', 'test/core/util/tracer_util.cc', @@ -722,7 +722,7 @@ 'test/core/util/parse_hexstring.cc', 'test/core/util/passthru_endpoint.cc', 'test/core/util/port.cc', - 'test/core/util/port_hermetic.cc', + 'test/core/util/port_isolated_runtime_environment.cc', 'test/core/util/port_server_client.cc', 'test/core/util/slice_splitter.cc', 'test/core/util/tracer_util.cc', diff --git a/test/core/util/BUILD b/test/core/util/BUILD index f92c0ff5489..89e07c83c78 100644 --- a/test/core/util/BUILD +++ b/test/core/util/BUILD @@ -53,7 +53,7 @@ grpc_cc_library( "parse_hexstring.cc", "passthru_endpoint.cc", "port.cc", - "port_hermetic.cc", + "port_isolated_runtime_environment.cc", "port_server_client.cc", "reconnect_server.cc", "slice_splitter.cc", diff --git a/test/core/util/port_hermetic.cc b/test/core/util/port_isolated_runtime_environment.cc similarity index 76% rename from test/core/util/port_hermetic.cc rename to test/core/util/port_isolated_runtime_environment.cc index b4d097f650d..5f0585e9fb4 100644 --- a/test/core/util/port_hermetic.cc +++ b/test/core/util/port_isolated_runtime_environment.cc @@ -16,13 +16,12 @@ * */ -/* When running tests hermeticly, i.e. running on remote machines, - * the framework takes a round-robin pick of a port within certain range. - * There is no need to recycle ports. +/* When running tests on remote machines, the framework takes a round-robin pick + * of a port within certain range. There is no need to recycle ports. */ #include "src/core/lib/iomgr/port.h" #include "test/core/util/test_config.h" -#if defined(GRPC_HERMETIC_TESTS) +#if defined(GRPC_PORT_ISOLATED_RUNTIME) #include "test/core/util/port.h" @@ -38,8 +37,6 @@ int grpc_pick_unused_port_or_die(void) { return allocated_port; } -void grpc_recycle_unused_port(int port) { - (void) port; -} +void grpc_recycle_unused_port(int port) { (void)port; } -#endif /* GRPC_HERMETIC_TESTS */ +#endif /* GRPC_PORT_ISOLATED_RUNTIME */ diff --git a/test/core/util/test_config.h b/test/core/util/test_config.h index ee60441653a..5b3d34799ee 100644 --- a/test/core/util/test_config.h +++ b/test/core/util/test_config.h @@ -33,7 +33,7 @@ gpr_timespec grpc_timeout_seconds_to_deadline(int64_t time_s); /* Converts a given timeout (in milliseconds) to a deadline. */ gpr_timespec grpc_timeout_milliseconds_to_deadline(int64_t time_ms); -#if !defined(GRPC_TEST_CUSTOM_PICK_PORT) && !defined(GRPC_HERMETIC_TESTS) +#if !defined(GRPC_TEST_CUSTOM_PICK_PORT) && !defined(GRPC_PORT_ISOLATED_RUNTIME) #define GRPC_TEST_PICK_PORT #endif diff --git a/tools/internal_ci/linux/grpc_bazel_on_foundry.sh b/tools/internal_ci/linux/grpc_bazel_on_foundry.sh index dc6df47ac81..e328be8aaba 100644 --- a/tools/internal_ci/linux/grpc_bazel_on_foundry.sh +++ b/tools/internal_ci/linux/grpc_bazel_on_foundry.sh @@ -15,6 +15,9 @@ set -ex +# A temporary solution to give Kokoro credentials. +# The file name 4321_grpc-testing-service needs to match auth_credential in +# the build config. mkdir -p ${KOKORO_KEYSTORE_DIR} cp ${KOKORO_GFILE_DIR}/GrpcTesting-d0eeee2db331.json ${KOKORO_KEYSTORE_DIR}/4321_grpc-testing-service @@ -48,5 +51,5 @@ source tools/internal_ci/helper_scripts/prepare_build_linux_rc --experimental_strict_action_env=true \ --experimental_remote_platform_override='properties:{name:"container-image" value:"docker://gcr.io/asci-toolchain/nosla-debian8-clang-fl@sha256:aa20628a902f06a11a015caa94b0432eb60690de2d2525bd046b9eea046f5d8a" }' \ --crosstool_top=@bazel_toolchains//configs/debian8_clang/0.2.0/bazel_0.7.0:toolchain \ - --define GRPC_HERMETIC_TESTS=1 \ + --define GRPC_PORT_ISOLATED_RUNTIME=1 \ -- //test/... diff --git a/tools/run_tests/generated/sources_and_headers.json b/tools/run_tests/generated/sources_and_headers.json index c957638181f..025077449fc 100644 --- a/tools/run_tests/generated/sources_and_headers.json +++ b/tools/run_tests/generated/sources_and_headers.json @@ -8932,8 +8932,7 @@ "test/core/util/mock_endpoint.h", "test/core/util/parse_hexstring.h", "test/core/util/passthru_endpoint.h", - "test/core/util/port.h", - "test/core/util/port_hermetic.cc", + "test/core/util/port.h", "test/core/util/port_server_client.h", "test/core/util/slice_splitter.h", "test/core/util/tracer_util.h", @@ -8967,6 +8966,7 @@ "test/core/util/passthru_endpoint.h", "test/core/util/port.cc", "test/core/util/port.h", + "test/core/util/port_isolated_runtime_environment.cc", "test/core/util/port_server_client.cc", "test/core/util/port_server_client.h", "test/core/util/slice_splitter.cc", From c05f0240e84ec77537ea6d576273840f14b9f0da Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 7 Nov 2017 17:19:57 +0100 Subject: [PATCH 23/39] fix nit --- test/distrib/cpp/run_distrib_test_cmake.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/test/distrib/cpp/run_distrib_test_cmake.sh b/test/distrib/cpp/run_distrib_test_cmake.sh index ead8cc10bc7..a9c081c2f5d 100755 --- a/test/distrib/cpp/run_distrib_test_cmake.sh +++ b/test/distrib/cpp/run_distrib_test_cmake.sh @@ -19,7 +19,6 @@ cd $(dirname $0)/../../.. echo "deb http://ftp.debian.org/debian jessie-backports main" | tee /etc/apt/sources.list.d/jessie-backports.list apt-get update -#apt-get install -t jessie-backports -y libc-ares-dev # we need specifically version 1.12 apt-get install -t jessie-backports -y libssl-dev # Install c-ares From 3c93a1982d34c23f685f148ce2d94f078bd8828a Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 7 Nov 2017 17:19:20 +0100 Subject: [PATCH 24/39] working cmake install on windows --- CMakeLists.txt | 1 + templates/CMakeLists.txt.template | 1 + test/distrib/cpp/run_distrib_test_cmake.bat | 64 +++++++++++++++++++++ 3 files changed, 66 insertions(+) create mode 100644 test/distrib/cpp/run_distrib_test_cmake.bat diff --git a/CMakeLists.txt b/CMakeLists.txt index ab57f662cf2..cf2b3cc82fa 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -235,6 +235,7 @@ if("${gRPC_SSL_PROVIDER}" STREQUAL "module") elseif("${gRPC_SSL_PROVIDER}" STREQUAL "package") find_package(OpenSSL REQUIRED) set(_gRPC_SSL_LIBRARIES ${OPENSSL_LIBRARIES}) + include_directories(${OPENSSL_INCLUDE_DIR}) set(_gRPC_FIND_SSL "if(NOT OPENSSL_FOUND)\n find_package(OpenSSL)\nendif()") endif() diff --git a/templates/CMakeLists.txt.template b/templates/CMakeLists.txt.template index 0be5d14cfef..f89371057f4 100644 --- a/templates/CMakeLists.txt.template +++ b/templates/CMakeLists.txt.template @@ -280,6 +280,7 @@ elseif("<%text>${gRPC_SSL_PROVIDER}" STREQUAL "package") find_package(OpenSSL REQUIRED) set(_gRPC_SSL_LIBRARIES <%text>${OPENSSL_LIBRARIES}) + include_directories(<%text>${OPENSSL_INCLUDE_DIR}) set(_gRPC_FIND_SSL "if(NOT OPENSSL_FOUND)\n find_package(OpenSSL)\nendif()") endif() diff --git a/test/distrib/cpp/run_distrib_test_cmake.bat b/test/distrib/cpp/run_distrib_test_cmake.bat new file mode 100644 index 00000000000..58d4a2a4c95 --- /dev/null +++ b/test/distrib/cpp/run_distrib_test_cmake.bat @@ -0,0 +1,64 @@ +@rem Copyright 2016 gRPC authors. +@rem +@rem Licensed under the Apache License, Version 2.0 (the "License"); +@rem you may not use this file except in compliance with the License. +@rem You may obtain a copy of the License at +@rem +@rem http://www.apache.org/licenses/LICENSE-2.0 +@rem +@rem Unless required by applicable law or agreed to in writing, software +@rem distributed under the License is distributed on an "AS IS" BASIS, +@rem WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +@rem See the License for the specific language governing permissions and +@rem limitations under the License. + +@rem enter this directory +cd /d %~dp0\..\..\.. + +@rem TODO(jtattermusch): use better install directory +@rem set INSTALL_DIR=%cd%/testinstall +set INSTALL_DIR=C:/testinstall + +cd third_party/zlib +mkdir cmake +cd cmake +cmake -DCMAKE_INSTALL_PREFIX=%INSTALL_DIR% .. +cmake --build . --config Release --target install || goto :error +cd ../../.. + +cd third_party/protobuf/cmake +mkdir build +cd build +cmake -DCMAKE_INSTALL_PREFIX=%INSTALL_DIR% -Dprotobuf_MSVC_STATIC_RUNTIME=OFF -Dprotobuf_BUILD_TESTS=OFF .. +cmake --build . --config Release --target install || goto :error +cd ../../../.. + +cd third_party/cares/cares +mkdir cmake +cd cmake +cmake -DCMAKE_INSTALL_PREFIX=%INSTALL_DIR% .. +cmake --build . --config Release --target install || goto :error +cd ../../../.. + +@rem TODO(jtattermusch): what do do with gflags +@rem cd third_party/gflags/cmake +@rem mkdir build +@rem cd build +@rem cmake -DCMAKE_INSTALL_PREFIX=%INSTALL_DIR% ../.. +@rem cmake --build . --config Release --target install || goto :error +@rem cd ../../../.. + + +@rem OpenSSL-Win32 and OpenSSL-Win64 can be downloaded from https://slproweb.com/products/Win32OpenSSL.html +cd cmake +mkdir build +cd build +cmake -DCMAKE_INSTALL_PREFIX=%INSTALL_DIR% -DOPENSSL_ROOT_DIR=C:/OpenSSL-Win32 -DOPENSSL_INCLUDE_DIR=C:/OpenSSL-Win32/include -DZLIB_LIBRARY=%INSTALL_DIR%/lib/zlibstatic.lib -DZLIB_INCLUDE_DIR=%INSTALL_DIR%/include -DgRPC_INSTALL=ON -DgRPC_BUILD_TESTS=OFF -DgRPC_PROTOBUF_PROVIDER=package -DgRPC_ZLIB_PROVIDER=package -DgRPC_CARES_PROVIDER=package -DgRPC_SSL_PROVIDER=package -DCMAKE_BUILD_TYPE=Release ../.. || goto :error +cmake --build . --config Release --target install || goto :error +cd ../.. + +goto :EOF + +:error +echo Failed! +exit /b %errorlevel% From 06251f8cd6a36f8406bf91874af2211de7e81441 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 9 Nov 2017 11:09:29 +0100 Subject: [PATCH 25/39] make helloworld CMakeLists compile on windows --- examples/cpp/helloworld/CMakeLists.txt | 23 +++++++++++++++++---- test/distrib/cpp/run_distrib_test_cmake.bat | 10 +++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/examples/cpp/helloworld/CMakeLists.txt b/examples/cpp/helloworld/CMakeLists.txt index 71a8db4f24f..a1869d21bbb 100644 --- a/examples/cpp/helloworld/CMakeLists.txt +++ b/examples/cpp/helloworld/CMakeLists.txt @@ -6,13 +6,28 @@ project(HelloWorld C CXX) if(NOT MSVC) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11") +else() + add_definitions(-D_WIN32_WINNT=0x600) endif() # Protobuf -set(protobuf_MODULE_COMPATIBLE TRUE) -find_package(protobuf CONFIG REQUIRED) +# NOTE: we cannot use "CONFIG" mode here because protobuf-config.cmake +# is broken when used with CMAKE_INSTALL_PREFIX +find_package(protobuf REQUIRED) message(STATUS "Using protobuf ${protobuf_VERSION}") +if(Protobuf_FOUND) + # Protobuf_FOUND is set for package type "CONFIG" + set(_PROTOBUF_LIBPROTOBUF protobuf::libprotobuf) + set(_PROTOBUF_PROTOC protobuf::protoc) +endif() +if(PROTOBUF_FOUND) + # PROTOBUF_FOUND is set for package type "MODULE" + set(_PROTOBUF_LIBPROTOBUF ${PROTOBUF_LIBRARIES}) + set(_PROTOBUF_PROTOC ${PROTOBUF_PROTOC_EXECUTABLE}) + include_directories(${PROTOBUF_INCLUDE_DIRS}) +endif() + # gRPC find_package(gRPC CONFIG REQUIRED) message(STATUS "Using gRPC ${gRPC_VERSION}") @@ -31,7 +46,7 @@ set(hw_grpc_srcs "${CMAKE_CURRENT_BINARY_DIR}/helloworld.grpc.pb.cc") set(hw_grpc_hdrs "${CMAKE_CURRENT_BINARY_DIR}/helloworld.grpc.pb.h") add_custom_command( OUTPUT "${hw_grpc_srcs}" "${hw_grpc_hdrs}" - COMMAND protobuf::protoc + COMMAND ${_PROTOBUF_PROTOC} ARGS --grpc_out "${CMAKE_CURRENT_BINARY_DIR}" -I "${hw_proto_path}" --plugin=protoc-gen-grpc="${gRPC_CPP_PLUGIN_EXECUTABLE}" "${hw_proto}" @@ -48,6 +63,6 @@ foreach(_target ${hw_proto_srcs} ${hw_grpc_srcs}) target_link_libraries(${_target} - protobuf::libprotobuf + ${_PROTOBUF_LIBPROTOBUF} gRPC::grpc++_unsecure) endforeach() diff --git a/test/distrib/cpp/run_distrib_test_cmake.bat b/test/distrib/cpp/run_distrib_test_cmake.bat index 58d4a2a4c95..ac4f6b77c00 100644 --- a/test/distrib/cpp/run_distrib_test_cmake.bat +++ b/test/distrib/cpp/run_distrib_test_cmake.bat @@ -57,6 +57,16 @@ cmake -DCMAKE_INSTALL_PREFIX=%INSTALL_DIR% -DOPENSSL_ROOT_DIR=C:/OpenSSL-Win32 - cmake --build . --config Release --target install || goto :error cd ../.. +# Build helloworld example using cmake +cd examples/cpp/helloworld +mkdir cmake +cd cmake +mkdir build +cd build +cmake -DCMAKE_INSTALL_PREFIX=%INSTALL_DIR% ../.. || goto :error +cmake --build . --config Release || goto :error +cd ../../../../.. + goto :EOF :error From ce7a23ba94ececce2c6bce499e1d45ec101acdca Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 14 Nov 2017 17:10:12 +0100 Subject: [PATCH 26/39] make runnable on kokoro --- test/distrib/cpp/run_distrib_test_cmake.bat | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/test/distrib/cpp/run_distrib_test_cmake.bat b/test/distrib/cpp/run_distrib_test_cmake.bat index ac4f6b77c00..48e14f98bbf 100644 --- a/test/distrib/cpp/run_distrib_test_cmake.bat +++ b/test/distrib/cpp/run_distrib_test_cmake.bat @@ -15,9 +15,15 @@ @rem enter this directory cd /d %~dp0\..\..\.. -@rem TODO(jtattermusch): use better install directory -@rem set INSTALL_DIR=%cd%/testinstall -set INSTALL_DIR=C:/testinstall +@rem Install into ./testinstall, but use absolute path and foward slashes +set INSTALL_DIR=%cd:\=/%/testinstall + +@rem Download OpenSSL-Win32 originally installed from https://slproweb.com/products/Win32OpenSSL.html +powershell -Command "(New-Object Net.WebClient).DownloadFile('https://storage.googleapis.com/grpc-testing.appspot.com/OpenSSL-Win32-1_1_0g.zip', 'OpenSSL-Win32.zip')" +powershell -Command "Add-Type -Assembly 'System.IO.Compression.FileSystem'; [System.IO.Compression.ZipFile]::ExtractToDirectory('OpenSSL-Win32.zip', '.');" + +@rem set absolute path to OpenSSL with forward slashes +set OPENSSL_DIR=%cd:\=/%/OpenSSL-Win32 cd third_party/zlib mkdir cmake @@ -48,12 +54,11 @@ cd ../../../.. @rem cmake --build . --config Release --target install || goto :error @rem cd ../../../.. - @rem OpenSSL-Win32 and OpenSSL-Win64 can be downloaded from https://slproweb.com/products/Win32OpenSSL.html cd cmake mkdir build cd build -cmake -DCMAKE_INSTALL_PREFIX=%INSTALL_DIR% -DOPENSSL_ROOT_DIR=C:/OpenSSL-Win32 -DOPENSSL_INCLUDE_DIR=C:/OpenSSL-Win32/include -DZLIB_LIBRARY=%INSTALL_DIR%/lib/zlibstatic.lib -DZLIB_INCLUDE_DIR=%INSTALL_DIR%/include -DgRPC_INSTALL=ON -DgRPC_BUILD_TESTS=OFF -DgRPC_PROTOBUF_PROVIDER=package -DgRPC_ZLIB_PROVIDER=package -DgRPC_CARES_PROVIDER=package -DgRPC_SSL_PROVIDER=package -DCMAKE_BUILD_TYPE=Release ../.. || goto :error +cmake -DCMAKE_INSTALL_PREFIX=%INSTALL_DIR% -DOPENSSL_ROOT_DIR=%OPENSSL_DIR% -DOPENSSL_INCLUDE_DIR=%OPENSSL_DIR%/include -DZLIB_LIBRARY=%INSTALL_DIR%/lib/zlibstatic.lib -DZLIB_INCLUDE_DIR=%INSTALL_DIR%/include -DgRPC_INSTALL=ON -DgRPC_BUILD_TESTS=OFF -DgRPC_PROTOBUF_PROVIDER=package -DgRPC_ZLIB_PROVIDER=package -DgRPC_CARES_PROVIDER=package -DgRPC_SSL_PROVIDER=package -DCMAKE_BUILD_TYPE=Release ../.. || goto :error cmake --build . --config Release --target install || goto :error cd ../.. From 27d254ad78ad462e0187cee0641ee5bb0be92d4d Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 14 Nov 2017 17:27:30 +0100 Subject: [PATCH 27/39] installing gflags not necessary --- test/distrib/cpp/run_distrib_test_cmake.bat | 8 -------- 1 file changed, 8 deletions(-) diff --git a/test/distrib/cpp/run_distrib_test_cmake.bat b/test/distrib/cpp/run_distrib_test_cmake.bat index 48e14f98bbf..f20721ce573 100644 --- a/test/distrib/cpp/run_distrib_test_cmake.bat +++ b/test/distrib/cpp/run_distrib_test_cmake.bat @@ -46,14 +46,6 @@ cmake -DCMAKE_INSTALL_PREFIX=%INSTALL_DIR% .. cmake --build . --config Release --target install || goto :error cd ../../../.. -@rem TODO(jtattermusch): what do do with gflags -@rem cd third_party/gflags/cmake -@rem mkdir build -@rem cd build -@rem cmake -DCMAKE_INSTALL_PREFIX=%INSTALL_DIR% ../.. -@rem cmake --build . --config Release --target install || goto :error -@rem cd ../../../.. - @rem OpenSSL-Win32 and OpenSSL-Win64 can be downloaded from https://slproweb.com/products/Win32OpenSSL.html cd cmake mkdir build From a5f05c70f1d71862959833c2a49e5ccdd68be27c Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Wed, 15 Nov 2017 10:48:05 +0100 Subject: [PATCH 28/39] avoid picking up ProtoC --- test/distrib/cpp/run_distrib_test_cmake.bat | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/distrib/cpp/run_distrib_test_cmake.bat b/test/distrib/cpp/run_distrib_test_cmake.bat index f20721ce573..047846b0f1a 100644 --- a/test/distrib/cpp/run_distrib_test_cmake.bat +++ b/test/distrib/cpp/run_distrib_test_cmake.bat @@ -15,6 +15,10 @@ @rem enter this directory cd /d %~dp0\..\..\.. +@rem TODO(jtattermusch): Kokoro has pre-installed protoc.exe in C:\Program Files\ProtoC and that directory +@rem is on PATH. To avoid picking up the older version protoc.exe, we change the path to something non-existent. +set PATH=%PATH:ProtoC=DontPickupProtoC% + @rem Install into ./testinstall, but use absolute path and foward slashes set INSTALL_DIR=%cd:\=/%/testinstall From b839d8c709250b348a446442edb87474737bea6a Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Wed, 15 Nov 2017 11:19:16 +0100 Subject: [PATCH 29/39] add distribtest target --- .../run_tests/artifacts/distribtest_targets.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/tools/run_tests/artifacts/distribtest_targets.py b/tools/run_tests/artifacts/distribtest_targets.py index 9959651b6cd..7ba0e0ebc90 100644 --- a/tools/run_tests/artifacts/distribtest_targets.py +++ b/tools/run_tests/artifacts/distribtest_targets.py @@ -49,7 +49,8 @@ def create_docker_jobspec(name, dockerfile_dir, shell_command, environ={}, def create_jobspec(name, cmdline, environ=None, shell=False, flake_retries=0, timeout_retries=0, - use_workspace=False): + use_workspace=False, + timeout_seconds=10*60): """Creates jobspec.""" environ = environ.copy() if use_workspace: @@ -60,7 +61,7 @@ def create_jobspec(name, cmdline, environ=None, shell=False, cmdline=cmdline, environ=environ, shortname='distribtest.%s' % (name), - timeout_seconds=10*60, + timeout_seconds=timeout_seconds, flake_retries=flake_retries, timeout_retries=timeout_retries, shell=shell) @@ -214,7 +215,10 @@ class CppDistribTest(object): """Tests Cpp make intall by building examples.""" def __init__(self, platform, arch, docker_suffix=None, testcase=None): - self.name = 'cpp_%s_%s_%s_%s' % (platform, arch, docker_suffix, testcase) + if platform == 'linux': + self.name = 'cpp_%s_%s_%s_%s' % (platform, arch, docker_suffix, testcase) + else: + self.name = 'cpp_%s_%s_%s' % (platform, arch, testcase) self.platform = platform self.arch = arch self.docker_suffix = docker_suffix @@ -231,6 +235,12 @@ class CppDistribTest(object): self.docker_suffix, self.arch), 'test/distrib/cpp/run_distrib_test_%s.sh' % self.testcase) + elif self.platform == 'windows': + return create_jobspec(self.name, + ['test\\distrib\\cpp\\run_distrib_test_%s.bat' % self.testcase], + environ={}, + timeout_seconds=30*60, + use_workspace=True) else: raise Exception("Not supported yet.") @@ -242,6 +252,7 @@ def targets(): """Gets list of supported targets""" return [CppDistribTest('linux', 'x64', 'jessie', 'routeguide'), CppDistribTest('linux', 'x64', 'jessie', 'cmake'), + CppDistribTest('windows', 'x86', testcase='cmake'), CSharpDistribTest('linux', 'x64', 'wheezy'), CSharpDistribTest('linux', 'x64', 'jessie'), CSharpDistribTest('linux', 'x86', 'jessie'), From f0620a37a5b417c4a35f18652432bfe2d14eeb3e Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 30 Nov 2017 17:45:11 +0100 Subject: [PATCH 30/39] fix linux distrib tests --- examples/cpp/helloworld/CMakeLists.txt | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/examples/cpp/helloworld/CMakeLists.txt b/examples/cpp/helloworld/CMakeLists.txt index a1869d21bbb..49684a13b04 100644 --- a/examples/cpp/helloworld/CMakeLists.txt +++ b/examples/cpp/helloworld/CMakeLists.txt @@ -13,19 +13,20 @@ endif() # Protobuf # NOTE: we cannot use "CONFIG" mode here because protobuf-config.cmake # is broken when used with CMAKE_INSTALL_PREFIX -find_package(protobuf REQUIRED) +find_package(Protobuf REQUIRED) message(STATUS "Using protobuf ${protobuf_VERSION}") if(Protobuf_FOUND) # Protobuf_FOUND is set for package type "CONFIG" set(_PROTOBUF_LIBPROTOBUF protobuf::libprotobuf) set(_PROTOBUF_PROTOC protobuf::protoc) -endif() -if(PROTOBUF_FOUND) +elseif(PROTOBUF_FOUND) # PROTOBUF_FOUND is set for package type "MODULE" set(_PROTOBUF_LIBPROTOBUF ${PROTOBUF_LIBRARIES}) set(_PROTOBUF_PROTOC ${PROTOBUF_PROTOC_EXECUTABLE}) include_directories(${PROTOBUF_INCLUDE_DIRS}) +else() + message(WARNING "Failed to locate libprotobuf and protoc!") endif() # gRPC From 7cb1f18dc27eacaf99aa059cdd429352b4a51d63 Mon Sep 17 00:00:00 2001 From: lyuxuan Date: Fri, 1 Dec 2017 11:40:10 -0800 Subject: [PATCH 31/39] Make load balancing policy name case insensitive --- doc/service_config.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/service_config.md b/doc/service_config.md index 0abbd7f324d..dd1cbc56300 100644 --- a/doc/service_config.md +++ b/doc/service_config.md @@ -12,7 +12,7 @@ The service config is a JSON string of the following form: ``` { - // Load balancing policy name. + // Load balancing policy name (case insensitive). // Currently, the only selectable client-side policy provided with gRPC // is 'round_robin', but third parties may add their own policies. // This field is optional; if unset, the default behavior is to pick From dd01db372866e6c3b1474f6cdbc1cd0ee94fa0ef Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Fri, 1 Dec 2017 12:44:29 -0800 Subject: [PATCH 32/39] Make comparison of LB policy name case-insensitive. --- src/core/ext/filters/client_channel/client_channel.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index 03c1b6f4bd5..58496dc2468 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -432,7 +432,7 @@ static void on_resolver_result_changed_locked(grpc_exec_ctx* exec_ctx, // once at any given time. lb_policy_name_changed = chand->info_lb_policy_name == nullptr || - strcmp(chand->info_lb_policy_name, lb_policy_name) != 0; + gpr_stricmp(chand->info_lb_policy_name, lb_policy_name) != 0; if (chand->lb_policy != nullptr && !lb_policy_name_changed) { // Continue using the same LB policy. Update with new addresses. lb_policy_updated = true; From e1e7db029f3dec84af97bb846f41d2fe3d613cc3 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Fri, 1 Dec 2017 13:01:01 -0800 Subject: [PATCH 33/39] Remove asserts from tls_gcc.h --- include/grpc/support/tls_gcc.h | 40 ---------------------------------- 1 file changed, 40 deletions(-) diff --git a/include/grpc/support/tls_gcc.h b/include/grpc/support/tls_gcc.h index 019acdf122e..1b91d22be12 100644 --- a/include/grpc/support/tls_gcc.h +++ b/include/grpc/support/tls_gcc.h @@ -26,44 +26,6 @@ /** Thread local storage based on gcc compiler primitives. #include tls.h to use this - and see that file for documentation */ -#ifndef NDEBUG - -struct gpr_gcc_thread_local { - intptr_t value; - bool* inited; -}; - -#define GPR_TLS_DECL(name) \ - static bool name##_inited = false; \ - static __thread struct gpr_gcc_thread_local name = {0, &(name##_inited)} - -#define gpr_tls_init(tls) \ - do { \ - GPR_ASSERT(*((tls)->inited) == false); \ - *((tls)->inited) = true; \ - } while (0) - -/** It is allowed to call gpr_tls_init after gpr_tls_destroy is called. */ -#define gpr_tls_destroy(tls) \ - do { \ - GPR_ASSERT(*((tls)->inited)); \ - *((tls)->inited) = false; \ - } while (0) - -#define gpr_tls_set(tls, new_value) \ - do { \ - GPR_ASSERT(*((tls)->inited)); \ - (tls)->value = (new_value); \ - } while (0) - -#define gpr_tls_get(tls) \ - ({ \ - GPR_ASSERT(*((tls)->inited)); \ - (tls)->value; \ - }) - -#else /* NDEBUG */ - struct gpr_gcc_thread_local { intptr_t value; }; @@ -80,6 +42,4 @@ struct gpr_gcc_thread_local { #define gpr_tls_set(tls, new_value) (((tls)->value) = (new_value)) #define gpr_tls_get(tls) ((tls)->value) -#endif /* NDEBUG */ - #endif /* GRPC_SUPPORT_TLS_GCC_H */ From df86101267fe579048d7efc068c95eca52e0ca23 Mon Sep 17 00:00:00 2001 From: Ken Payson Date: Fri, 1 Dec 2017 15:58:04 -0800 Subject: [PATCH 34/39] Initialize last sent ping time --- .../ext/transport/chttp2/transport/chttp2_transport.cc | 1 + src/core/ext/transport/chttp2/transport/writing.cc | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc index 01a16955d93..5a7830b0c07 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc @@ -549,6 +549,7 @@ static void init_transport(grpc_exec_ctx* exec_ctx, grpc_chttp2_transport* t, /* No pings allowed before receiving a header or data frame. */ t->ping_state.pings_before_data_required = 0; t->ping_state.is_delayed_ping_timer_set = false; + t->ping_state.last_ping_sent_time = GRPC_MILLIS_INF_PAST; t->ping_recv_state.last_ping_recv_time = GRPC_MILLIS_INF_PAST; t->ping_recv_state.ping_strikes = 0; diff --git a/src/core/ext/transport/chttp2/transport/writing.cc b/src/core/ext/transport/chttp2/transport/writing.cc index 15869b8880c..204b5a77087 100644 --- a/src/core/ext/transport/chttp2/transport/writing.cc +++ b/src/core/ext/transport/chttp2/transport/writing.cc @@ -81,8 +81,11 @@ static void maybe_initiate_ping(grpc_exec_ctx* exec_ctx, /* not enough elapsed time between successive pings */ if (grpc_http_trace.enabled() || grpc_bdp_estimator_trace.enabled()) { gpr_log(GPR_DEBUG, - "%s: Ping delayed [%p]: not enough time elapsed since last ping", - t->is_client ? "CLIENT" : "SERVER", t->peer_string); + "%s: Ping delayed [%p]: not enough time elapsed since last ping. " + " Last ping %f: Next ping %f: Now %f", + t->is_client ? "CLIENT" : "SERVER", t->peer_string, + (double)t->ping_state.last_ping_sent_time, + (double)next_allowed_ping, (double)now); } if (!t->ping_state.is_delayed_ping_timer_set) { t->ping_state.is_delayed_ping_timer_set = true; @@ -91,6 +94,7 @@ static void maybe_initiate_ping(grpc_exec_ctx* exec_ctx, } return; } + pq->inflight_id = t->ping_ctr; t->ping_ctr++; GRPC_CLOSURE_LIST_SCHED(exec_ctx, &pq->lists[GRPC_CHTTP2_PCL_INITIATE]); From 6389457ed2ef7af141a3723c3eb82a1cdc81293b Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Thu, 30 Nov 2017 10:16:21 -0800 Subject: [PATCH 35/39] Adjust stream cancellation point and fix races in sync client --- test/cpp/qps/client_sync.cc | 193 ++++++++++++++++++++++-------------- 1 file changed, 121 insertions(+), 72 deletions(-) diff --git a/test/cpp/qps/client_sync.cc b/test/cpp/qps/client_sync.cc index 9f20b148eb3..cb0945b05b8 100644 --- a/test/cpp/qps/client_sync.cc +++ b/test/cpp/qps/client_sync.cc @@ -62,11 +62,13 @@ class SynchronousClient virtual ~SynchronousClient(){}; - virtual void InitThreadFuncImpl(size_t thread_idx) = 0; + virtual bool InitThreadFuncImpl(size_t thread_idx) = 0; virtual bool ThreadFuncImpl(HistogramEntry* entry, size_t thread_idx) = 0; void ThreadFunc(size_t thread_idx, Thread* t) override { - InitThreadFuncImpl(thread_idx); + if (!InitThreadFuncImpl(thread_idx)) { + return; + } for (;;) { // run the loop body HistogramEntry entry; @@ -109,9 +111,6 @@ class SynchronousClient size_t num_threads_; std::vector responses_; - - private: - void DestroyMultithreading() override final { EndThreads(); } }; class SynchronousUnaryClient final : public SynchronousClient { @@ -122,7 +121,7 @@ class SynchronousUnaryClient final : public SynchronousClient { } ~SynchronousUnaryClient() {} - void InitThreadFuncImpl(size_t thread_idx) override {} + bool InitThreadFuncImpl(size_t thread_idx) override { return true; } bool ThreadFuncImpl(HistogramEntry* entry, size_t thread_idx) override { if (!WaitToIssue(thread_idx)) { @@ -140,6 +139,9 @@ class SynchronousUnaryClient final : public SynchronousClient { entry->set_status(s.error_code()); return true; } + + private: + void DestroyMultithreading() override final { EndThreads(); } }; template @@ -149,31 +151,30 @@ class SynchronousStreamingClient : public SynchronousClient { : SynchronousClient(config), context_(num_threads_), stream_(num_threads_), + stream_mu_(num_threads_), + shutdown_(num_threads_), messages_per_stream_(config.messages_per_stream()), messages_issued_(num_threads_) { StartThreads(num_threads_); } virtual ~SynchronousStreamingClient() { - std::vector cleanup_threads; - for (size_t i = 0; i < num_threads_; i++) { - cleanup_threads.emplace_back([this, i]() { - auto stream = &stream_[i]; - if (*stream) { - // forcibly cancel the streams, then finish - context_[i].TryCancel(); - (*stream)->Finish().IgnoreError(); - // don't log any error message on !ok since this was canceled - } - }); - } - for (auto& th : cleanup_threads) { - th.join(); - } + OnAllStreams([](ClientContext* ctx, StreamType* s) -> bool { + // don't log any kind of error since we might have canceled it + s->Finish().IgnoreError(); + return true; + }); } protected: std::vector context_; std::vector> stream_; + // stream_mu_ is only needed when changing an element of stream_ or context_ + std::vector stream_mu_; + struct Bool { + bool val; + Bool() : val(false) {} + }; + std::vector shutdown_; const int messages_per_stream_; std::vector messages_issued_; @@ -185,9 +186,34 @@ class SynchronousStreamingClient : public SynchronousClient { gpr_log(GPR_ERROR, "Stream %" PRIuPTR " received an error %s", thread_idx, s.error_message().c_str()); } + // Lock the stream_mu_ now because the client context could change + std::lock_guard l(stream_mu_[thread_idx]); context_[thread_idx].~ClientContext(); new (&context_[thread_idx]) ClientContext(); } + void OnAllStreams(std::function cleaner) { + std::vector cleanup_threads; + for (size_t i = 0; i < num_threads_; i++) { + cleanup_threads.emplace_back([this, i, cleaner]() { + std::lock_guard l(stream_mu_[i]); + if (stream_[i]) { + shutdown_[i].val = cleaner(&context_[i], stream_[i].get()); + } + }); + } + for (auto& th : cleanup_threads) { + th.join(); + } + } + + private: + void DestroyMultithreading() override final { + OnAllStreams([](ClientContext* ctx, StreamType* s) -> bool { + ctx->TryCancel(); + return true; + }); + EndThreads(); + } }; class SynchronousStreamingPingPongClient final @@ -197,24 +223,24 @@ class SynchronousStreamingPingPongClient final SynchronousStreamingPingPongClient(const ClientConfig& config) : SynchronousStreamingClient(config) {} ~SynchronousStreamingPingPongClient() { - std::vector cleanup_threads; - for (size_t i = 0; i < num_threads_; i++) { - cleanup_threads.emplace_back([this, i]() { - auto stream = &stream_[i]; - if (*stream) { - (*stream)->WritesDone(); - } - }); - } - for (auto& th : cleanup_threads) { - th.join(); - } + OnAllStreams( + [](ClientContext* ctx, + grpc::ClientReaderWriter* s) -> bool { + s->WritesDone(); + return true; + }); } - void InitThreadFuncImpl(size_t thread_idx) override { + bool InitThreadFuncImpl(size_t thread_idx) override { auto* stub = channels_[thread_idx % channels_.size()].get_stub(); - stream_[thread_idx] = stub->StreamingCall(&context_[thread_idx]); + std::lock_guard l(stream_mu_[thread_idx]); + if (!shutdown_[thread_idx].val) { + stream_[thread_idx] = stub->StreamingCall(&context_[thread_idx]); + } else { + return false; + } messages_issued_[thread_idx] = 0; + return true; } bool ThreadFuncImpl(HistogramEntry* entry, size_t thread_idx) override { @@ -239,7 +265,13 @@ class SynchronousStreamingPingPongClient final stream_[thread_idx]->WritesDone(); FinishStream(entry, thread_idx); auto* stub = channels_[thread_idx % channels_.size()].get_stub(); - stream_[thread_idx] = stub->StreamingCall(&context_[thread_idx]); + std::lock_guard l(stream_mu_[thread_idx]); + if (!shutdown_[thread_idx].val) { + stream_[thread_idx] = stub->StreamingCall(&context_[thread_idx]); + } else { + stream_[thread_idx].reset(); + return false; + } messages_issued_[thread_idx] = 0; return true; } @@ -251,25 +283,24 @@ class SynchronousStreamingFromClientClient final SynchronousStreamingFromClientClient(const ClientConfig& config) : SynchronousStreamingClient(config), last_issue_(num_threads_) {} ~SynchronousStreamingFromClientClient() { - std::vector cleanup_threads; - for (size_t i = 0; i < num_threads_; i++) { - cleanup_threads.emplace_back([this, i]() { - auto stream = &stream_[i]; - if (*stream) { - (*stream)->WritesDone(); - } - }); - } - for (auto& th : cleanup_threads) { - th.join(); - } + OnAllStreams( + [](ClientContext* ctx, grpc::ClientWriter* s) -> bool { + s->WritesDone(); + return true; + }); } - void InitThreadFuncImpl(size_t thread_idx) override { + bool InitThreadFuncImpl(size_t thread_idx) override { auto* stub = channels_[thread_idx % channels_.size()].get_stub(); - stream_[thread_idx] = stub->StreamingFromClient(&context_[thread_idx], - &responses_[thread_idx]); + std::lock_guard l(stream_mu_[thread_idx]); + if (!shutdown_[thread_idx].val) { + stream_[thread_idx] = stub->StreamingFromClient(&context_[thread_idx], + &responses_[thread_idx]); + } else { + return false; + } last_issue_[thread_idx] = UsageTimer::Now(); + return true; } bool ThreadFuncImpl(HistogramEntry* entry, size_t thread_idx) override { @@ -287,8 +318,14 @@ class SynchronousStreamingFromClientClient final stream_[thread_idx]->WritesDone(); FinishStream(entry, thread_idx); auto* stub = channels_[thread_idx % channels_.size()].get_stub(); - stream_[thread_idx] = stub->StreamingFromClient(&context_[thread_idx], - &responses_[thread_idx]); + std::lock_guard l(stream_mu_[thread_idx]); + if (!shutdown_[thread_idx].val) { + stream_[thread_idx] = stub->StreamingFromClient(&context_[thread_idx], + &responses_[thread_idx]); + } else { + stream_[thread_idx].reset(); + return false; + } return true; } @@ -301,11 +338,17 @@ class SynchronousStreamingFromServerClient final public: SynchronousStreamingFromServerClient(const ClientConfig& config) : SynchronousStreamingClient(config), last_recv_(num_threads_) {} - void InitThreadFuncImpl(size_t thread_idx) override { + bool InitThreadFuncImpl(size_t thread_idx) override { auto* stub = channels_[thread_idx % channels_.size()].get_stub(); - stream_[thread_idx] = - stub->StreamingFromServer(&context_[thread_idx], request_); + std::lock_guard l(stream_mu_[thread_idx]); + if (!shutdown_[thread_idx].val) { + stream_[thread_idx] = + stub->StreamingFromServer(&context_[thread_idx], request_); + } else { + return false; + } last_recv_[thread_idx] = UsageTimer::Now(); + return true; } bool ThreadFuncImpl(HistogramEntry* entry, size_t thread_idx) override { GPR_TIMER_SCOPE("SynchronousStreamingFromServerClient::ThreadFunc", 0); @@ -317,8 +360,14 @@ class SynchronousStreamingFromServerClient final } FinishStream(entry, thread_idx); auto* stub = channels_[thread_idx % channels_.size()].get_stub(); - stream_[thread_idx] = - stub->StreamingFromServer(&context_[thread_idx], request_); + std::lock_guard l(stream_mu_[thread_idx]); + if (!shutdown_[thread_idx].val) { + stream_[thread_idx] = + stub->StreamingFromServer(&context_[thread_idx], request_); + } else { + stream_[thread_idx].reset(); + return false; + } return true; } @@ -333,23 +382,23 @@ class SynchronousStreamingBothWaysClient final SynchronousStreamingBothWaysClient(const ClientConfig& config) : SynchronousStreamingClient(config) {} ~SynchronousStreamingBothWaysClient() { - std::vector cleanup_threads; - for (size_t i = 0; i < num_threads_; i++) { - cleanup_threads.emplace_back([this, i]() { - auto stream = &stream_[i]; - if (*stream) { - (*stream)->WritesDone(); - } - }); - } - for (auto& th : cleanup_threads) { - th.join(); - } + OnAllStreams( + [](ClientContext* ctx, + grpc::ClientReaderWriter* s) -> bool { + s->WritesDone(); + return true; + }); } - void InitThreadFuncImpl(size_t thread_idx) override { + bool InitThreadFuncImpl(size_t thread_idx) override { auto* stub = channels_[thread_idx % channels_.size()].get_stub(); - stream_[thread_idx] = stub->StreamingBothWays(&context_[thread_idx]); + std::lock_guard l(stream_mu_[thread_idx]); + if (!shutdown_[thread_idx].val) { + stream_[thread_idx] = stub->StreamingBothWays(&context_[thread_idx]); + } else { + return false; + } + return true; } bool ThreadFuncImpl(HistogramEntry* entry, size_t thread_idx) override { // TODO (vjpai): Do this From c8dd4c513908568c68c2b30d49d0303bb764bf17 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Mon, 4 Dec 2017 01:38:00 -0800 Subject: [PATCH 36/39] Remove some unneeded and spammy logs --- test/cpp/qps/client_sync.cc | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/test/cpp/qps/client_sync.cc b/test/cpp/qps/client_sync.cc index cb0945b05b8..c61e621dc4b 100644 --- a/test/cpp/qps/client_sync.cc +++ b/test/cpp/qps/client_sync.cc @@ -74,9 +74,6 @@ class SynchronousClient HistogramEntry entry; const bool thread_still_ok = ThreadFuncImpl(&entry, thread_idx); t->UpdateHistogram(&entry); - if (!thread_still_ok) { - gpr_log(GPR_ERROR, "Finishing client thread due to RPC error"); - } if (!thread_still_ok || ThreadCompleted()) { return; } @@ -170,6 +167,7 @@ class SynchronousStreamingClient : public SynchronousClient { std::vector> stream_; // stream_mu_ is only needed when changing an element of stream_ or context_ std::vector stream_mu_; + // use struct Bool rather than bool because vector is not concurrent struct Bool { bool val; Bool() : val(false) {} @@ -183,8 +181,11 @@ class SynchronousStreamingClient : public SynchronousClient { // don't set the value since the stream is failed and shouldn't be timed entry->set_status(s.error_code()); if (!s.ok()) { - gpr_log(GPR_ERROR, "Stream %" PRIuPTR " received an error %s", thread_idx, - s.error_message().c_str()); + std::lock_guard l(stream_mu_[thread_idx]); + if (!shutdown_[thread_idx].val) { + gpr_log(GPR_ERROR, "Stream %" PRIuPTR " received an error %s", + thread_idx, s.error_message().c_str()); + } } // Lock the stream_mu_ now because the client context could change std::lock_guard l(stream_mu_[thread_idx]); From 083b9be1a214f2ff67581f186684313c8d6b75fe Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Mon, 4 Dec 2017 09:56:28 -0800 Subject: [PATCH 37/39] Make all-streams op about cleanup only and replace a lambda with a virtual --- test/cpp/qps/client_sync.cc | 89 +++++++++++++++++++++---------------- 1 file changed, 50 insertions(+), 39 deletions(-) diff --git a/test/cpp/qps/client_sync.cc b/test/cpp/qps/client_sync.cc index c61e621dc4b..20aa5f54353 100644 --- a/test/cpp/qps/client_sync.cc +++ b/test/cpp/qps/client_sync.cc @@ -60,7 +60,7 @@ class SynchronousClient SetupLoadTest(config, num_threads_); } - virtual ~SynchronousClient(){}; + virtual ~SynchronousClient() {} virtual bool InitThreadFuncImpl(size_t thread_idx) = 0; virtual bool ThreadFuncImpl(HistogramEntry* entry, size_t thread_idx) = 0; @@ -154,13 +154,7 @@ class SynchronousStreamingClient : public SynchronousClient { messages_issued_(num_threads_) { StartThreads(num_threads_); } - virtual ~SynchronousStreamingClient() { - OnAllStreams([](ClientContext* ctx, StreamType* s) -> bool { - // don't log any kind of error since we might have canceled it - s->Finish().IgnoreError(); - return true; - }); - } + virtual ~SynchronousStreamingClient() {} protected: std::vector context_; @@ -192,13 +186,19 @@ class SynchronousStreamingClient : public SynchronousClient { context_[thread_idx].~ClientContext(); new (&context_[thread_idx]) ClientContext(); } - void OnAllStreams(std::function cleaner) { + + virtual void CleanStream(size_t thread_idx) { + context_[thread_idx].TryCancel(); + } + + void CleanupAllStreams() { std::vector cleanup_threads; for (size_t i = 0; i < num_threads_; i++) { - cleanup_threads.emplace_back([this, i, cleaner]() { + cleanup_threads.emplace_back([this, i] { std::lock_guard l(stream_mu_[i]); + shutdown_[i].val = true; if (stream_[i]) { - shutdown_[i].val = cleaner(&context_[i], stream_[i].get()); + CleanStream(i); } }); } @@ -206,13 +206,9 @@ class SynchronousStreamingClient : public SynchronousClient { th.join(); } } - private: void DestroyMultithreading() override final { - OnAllStreams([](ClientContext* ctx, StreamType* s) -> bool { - ctx->TryCancel(); - return true; - }); + CleanupAllStreams(); EndThreads(); } }; @@ -224,14 +220,9 @@ class SynchronousStreamingPingPongClient final SynchronousStreamingPingPongClient(const ClientConfig& config) : SynchronousStreamingClient(config) {} ~SynchronousStreamingPingPongClient() { - OnAllStreams( - [](ClientContext* ctx, - grpc::ClientReaderWriter* s) -> bool { - s->WritesDone(); - return true; - }); + CleanupAllStreams(); } - + private: bool InitThreadFuncImpl(size_t thread_idx) override { auto* stub = channels_[thread_idx % channels_.size()].get_stub(); std::lock_guard l(stream_mu_[thread_idx]); @@ -276,6 +267,12 @@ class SynchronousStreamingPingPongClient final messages_issued_[thread_idx] = 0; return true; } + + void CleanStream(size_t thread_idx) override { + stream_[thread_idx]->WritesDone(); + // Don't log any kind of error since we may have canceled this + stream_[thread_idx]->Finish().IgnoreError(); + } }; class SynchronousStreamingFromClientClient final @@ -284,13 +281,12 @@ class SynchronousStreamingFromClientClient final SynchronousStreamingFromClientClient(const ClientConfig& config) : SynchronousStreamingClient(config), last_issue_(num_threads_) {} ~SynchronousStreamingFromClientClient() { - OnAllStreams( - [](ClientContext* ctx, grpc::ClientWriter* s) -> bool { - s->WritesDone(); - return true; - }); + CleanupAllStreams(); } + private: + std::vector last_issue_; + bool InitThreadFuncImpl(size_t thread_idx) override { auto* stub = channels_[thread_idx % channels_.size()].get_stub(); std::lock_guard l(stream_mu_[thread_idx]); @@ -330,8 +326,11 @@ class SynchronousStreamingFromClientClient final return true; } - private: - std::vector last_issue_; + void CleanStream(size_t thread_idx) override { + stream_[thread_idx]->WritesDone(); + // Don't log any kind of error since we may have canceled this + stream_[thread_idx]->Finish().IgnoreError(); + } }; class SynchronousStreamingFromServerClient final @@ -339,6 +338,13 @@ class SynchronousStreamingFromServerClient final public: SynchronousStreamingFromServerClient(const ClientConfig& config) : SynchronousStreamingClient(config), last_recv_(num_threads_) {} + ~SynchronousStreamingFromServerClient() { + CleanupAllStreams(); + } + + private: + std::vector last_recv_; + bool InitThreadFuncImpl(size_t thread_idx) override { auto* stub = channels_[thread_idx % channels_.size()].get_stub(); std::lock_guard l(stream_mu_[thread_idx]); @@ -351,6 +357,7 @@ class SynchronousStreamingFromServerClient final last_recv_[thread_idx] = UsageTimer::Now(); return true; } + bool ThreadFuncImpl(HistogramEntry* entry, size_t thread_idx) override { GPR_TIMER_SCOPE("SynchronousStreamingFromServerClient::ThreadFunc", 0); if (stream_[thread_idx]->Read(&responses_[thread_idx])) { @@ -372,8 +379,10 @@ class SynchronousStreamingFromServerClient final return true; } - private: - std::vector last_recv_; + void CleanStream(size_t thread_idx) override { + // Don't log any kind of error since we may have canceled this + stream_[thread_idx]->Finish().IgnoreError(); + } }; class SynchronousStreamingBothWaysClient final @@ -383,14 +392,9 @@ class SynchronousStreamingBothWaysClient final SynchronousStreamingBothWaysClient(const ClientConfig& config) : SynchronousStreamingClient(config) {} ~SynchronousStreamingBothWaysClient() { - OnAllStreams( - [](ClientContext* ctx, - grpc::ClientReaderWriter* s) -> bool { - s->WritesDone(); - return true; - }); + CleanupAllStreams(); } - + private: bool InitThreadFuncImpl(size_t thread_idx) override { auto* stub = channels_[thread_idx % channels_.size()].get_stub(); std::lock_guard l(stream_mu_[thread_idx]); @@ -401,10 +405,17 @@ class SynchronousStreamingBothWaysClient final } return true; } + bool ThreadFuncImpl(HistogramEntry* entry, size_t thread_idx) override { // TODO (vjpai): Do this return true; } + + void CleanStream(size_t thread_idx) override { + stream_[thread_idx]->WritesDone(); + // Don't log any kind of error since we may have canceled this + stream_[thread_idx]->Finish().IgnoreError(); + } }; std::unique_ptr CreateSynchronousClient(const ClientConfig& config) { From f230ffd476fb095f0143a1b673320d93dfab8f6a Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Mon, 4 Dec 2017 09:58:44 -0800 Subject: [PATCH 38/39] clang-format --- test/cpp/qps/client_sync.cc | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/test/cpp/qps/client_sync.cc b/test/cpp/qps/client_sync.cc index 20aa5f54353..69ba8662336 100644 --- a/test/cpp/qps/client_sync.cc +++ b/test/cpp/qps/client_sync.cc @@ -196,9 +196,9 @@ class SynchronousStreamingClient : public SynchronousClient { for (size_t i = 0; i < num_threads_; i++) { cleanup_threads.emplace_back([this, i] { std::lock_guard l(stream_mu_[i]); - shutdown_[i].val = true; + shutdown_[i].val = true; if (stream_[i]) { - CleanStream(i); + CleanStream(i); } }); } @@ -206,6 +206,7 @@ class SynchronousStreamingClient : public SynchronousClient { th.join(); } } + private: void DestroyMultithreading() override final { CleanupAllStreams(); @@ -219,9 +220,8 @@ class SynchronousStreamingPingPongClient final public: SynchronousStreamingPingPongClient(const ClientConfig& config) : SynchronousStreamingClient(config) {} - ~SynchronousStreamingPingPongClient() { - CleanupAllStreams(); - } + ~SynchronousStreamingPingPongClient() { CleanupAllStreams(); } + private: bool InitThreadFuncImpl(size_t thread_idx) override { auto* stub = channels_[thread_idx % channels_.size()].get_stub(); @@ -280,9 +280,7 @@ class SynchronousStreamingFromClientClient final public: SynchronousStreamingFromClientClient(const ClientConfig& config) : SynchronousStreamingClient(config), last_issue_(num_threads_) {} - ~SynchronousStreamingFromClientClient() { - CleanupAllStreams(); - } + ~SynchronousStreamingFromClientClient() { CleanupAllStreams(); } private: std::vector last_issue_; @@ -338,9 +336,7 @@ class SynchronousStreamingFromServerClient final public: SynchronousStreamingFromServerClient(const ClientConfig& config) : SynchronousStreamingClient(config), last_recv_(num_threads_) {} - ~SynchronousStreamingFromServerClient() { - CleanupAllStreams(); - } + ~SynchronousStreamingFromServerClient() { CleanupAllStreams(); } private: std::vector last_recv_; @@ -391,9 +387,8 @@ class SynchronousStreamingBothWaysClient final public: SynchronousStreamingBothWaysClient(const ClientConfig& config) : SynchronousStreamingClient(config) {} - ~SynchronousStreamingBothWaysClient() { - CleanupAllStreams(); - } + ~SynchronousStreamingBothWaysClient() { CleanupAllStreams(); } + private: bool InitThreadFuncImpl(size_t thread_idx) override { auto* stub = channels_[thread_idx % channels_.size()].get_stub(); From c6587ca11b3b769957f670ab08bbbc1036116523 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Mon, 4 Dec 2017 10:15:19 -0800 Subject: [PATCH 39/39] Reintroduce lambdas --- test/cpp/qps/client_sync.cc | 60 +++++++++++++++---------------------- 1 file changed, 24 insertions(+), 36 deletions(-) diff --git a/test/cpp/qps/client_sync.cc b/test/cpp/qps/client_sync.cc index 69ba8662336..82a3f0042d1 100644 --- a/test/cpp/qps/client_sync.cc +++ b/test/cpp/qps/client_sync.cc @@ -154,7 +154,12 @@ class SynchronousStreamingClient : public SynchronousClient { messages_issued_(num_threads_) { StartThreads(num_threads_); } - virtual ~SynchronousStreamingClient() {} + virtual ~SynchronousStreamingClient() { + CleanupAllStreams([this](size_t thread_idx) { + // Don't log any kind of error since we may have canceled this + stream_[thread_idx]->Finish().IgnoreError(); + }); + } protected: std::vector context_; @@ -187,18 +192,14 @@ class SynchronousStreamingClient : public SynchronousClient { new (&context_[thread_idx]) ClientContext(); } - virtual void CleanStream(size_t thread_idx) { - context_[thread_idx].TryCancel(); - } - - void CleanupAllStreams() { + void CleanupAllStreams(std::function cleaner) { std::vector cleanup_threads; for (size_t i = 0; i < num_threads_; i++) { - cleanup_threads.emplace_back([this, i] { + cleanup_threads.emplace_back([this, i, cleaner] { std::lock_guard l(stream_mu_[i]); shutdown_[i].val = true; if (stream_[i]) { - CleanStream(i); + cleaner(i); } }); } @@ -209,7 +210,8 @@ class SynchronousStreamingClient : public SynchronousClient { private: void DestroyMultithreading() override final { - CleanupAllStreams(); + CleanupAllStreams( + [this](size_t thread_idx) { context_[thread_idx].TryCancel(); }); EndThreads(); } }; @@ -220,7 +222,10 @@ class SynchronousStreamingPingPongClient final public: SynchronousStreamingPingPongClient(const ClientConfig& config) : SynchronousStreamingClient(config) {} - ~SynchronousStreamingPingPongClient() { CleanupAllStreams(); } + ~SynchronousStreamingPingPongClient() { + CleanupAllStreams( + [this](size_t thread_idx) { stream_[thread_idx]->WritesDone(); }); + } private: bool InitThreadFuncImpl(size_t thread_idx) override { @@ -267,12 +272,6 @@ class SynchronousStreamingPingPongClient final messages_issued_[thread_idx] = 0; return true; } - - void CleanStream(size_t thread_idx) override { - stream_[thread_idx]->WritesDone(); - // Don't log any kind of error since we may have canceled this - stream_[thread_idx]->Finish().IgnoreError(); - } }; class SynchronousStreamingFromClientClient final @@ -280,7 +279,10 @@ class SynchronousStreamingFromClientClient final public: SynchronousStreamingFromClientClient(const ClientConfig& config) : SynchronousStreamingClient(config), last_issue_(num_threads_) {} - ~SynchronousStreamingFromClientClient() { CleanupAllStreams(); } + ~SynchronousStreamingFromClientClient() { + CleanupAllStreams( + [this](size_t thread_idx) { stream_[thread_idx]->WritesDone(); }); + } private: std::vector last_issue_; @@ -323,12 +325,6 @@ class SynchronousStreamingFromClientClient final } return true; } - - void CleanStream(size_t thread_idx) override { - stream_[thread_idx]->WritesDone(); - // Don't log any kind of error since we may have canceled this - stream_[thread_idx]->Finish().IgnoreError(); - } }; class SynchronousStreamingFromServerClient final @@ -336,7 +332,7 @@ class SynchronousStreamingFromServerClient final public: SynchronousStreamingFromServerClient(const ClientConfig& config) : SynchronousStreamingClient(config), last_recv_(num_threads_) {} - ~SynchronousStreamingFromServerClient() { CleanupAllStreams(); } + ~SynchronousStreamingFromServerClient() {} private: std::vector last_recv_; @@ -374,11 +370,6 @@ class SynchronousStreamingFromServerClient final } return true; } - - void CleanStream(size_t thread_idx) override { - // Don't log any kind of error since we may have canceled this - stream_[thread_idx]->Finish().IgnoreError(); - } }; class SynchronousStreamingBothWaysClient final @@ -387,7 +378,10 @@ class SynchronousStreamingBothWaysClient final public: SynchronousStreamingBothWaysClient(const ClientConfig& config) : SynchronousStreamingClient(config) {} - ~SynchronousStreamingBothWaysClient() { CleanupAllStreams(); } + ~SynchronousStreamingBothWaysClient() { + CleanupAllStreams( + [this](size_t thread_idx) { stream_[thread_idx]->WritesDone(); }); + } private: bool InitThreadFuncImpl(size_t thread_idx) override { @@ -405,12 +399,6 @@ class SynchronousStreamingBothWaysClient final // TODO (vjpai): Do this return true; } - - void CleanStream(size_t thread_idx) override { - stream_[thread_idx]->WritesDone(); - // Don't log any kind of error since we may have canceled this - stream_[thread_idx]->Finish().IgnoreError(); - } }; std::unique_ptr CreateSynchronousClient(const ClientConfig& config) {