From f2a5ed081a392149c189aab0579683dffc89f643 Mon Sep 17 00:00:00 2001 From: Ruchir Rastogi Date: Wed, 10 Jun 2020 20:43:53 -0700 Subject: [PATCH] Do not truncate AStatsEvent buffers Pushed atoms do not need to be truncated because only the bytes added to the buffer are written to the socket. Pulled atoms do not need to be truncated because within stats_pull_atom_callback.cpp, we only copy the valid parts of the buffer to the StatsEventParcel object. This improves performance by avoiding a needless call to realloc. + removed dead benchmarking code Test: m libstatssocket Test: atest libstatssocket_test Test: atest GtsStatsdHostTestCases Bug: 158717786 Change-Id: I6965f8832758203ca566336ba12d0acaf5f756d5 --- libstats/socket/Android.bp | 21 +------- libstats/socket/benchmark/main.cpp | 19 ------- .../benchmark/stats_event_benchmark.cpp | 54 ------------------- libstats/socket/include/stats_event.h | 3 -- libstats/socket/stats_event.c | 13 ----- libstats/socket/tests/stats_event_test.cpp | 2 +- 6 files changed, 2 insertions(+), 110 deletions(-) delete mode 100644 libstats/socket/benchmark/main.cpp delete mode 100644 libstats/socket/benchmark/stats_event_benchmark.cpp diff --git a/libstats/socket/Android.bp b/libstats/socket/Android.bp index 2bf0261b2..bf79ea244 100644 --- a/libstats/socket/Android.bp +++ b/libstats/socket/Android.bp @@ -89,25 +89,6 @@ cc_library_headers { min_sdk_version: "29", } -cc_benchmark { - name: "libstatssocket_benchmark", - srcs: [ - "benchmark/main.cpp", - "benchmark/stats_event_benchmark.cpp", - ], - cflags: [ - "-Wall", - "-Werror", - ], - static_libs: [ - "libstatssocket_private", - ], - shared_libs: [ - "libcutils", - "libgtest_prod", - ], -} - cc_test { name: "libstatssocket_test", srcs: [ @@ -128,7 +109,7 @@ cc_test { ], test_suites: ["device-tests", "mts"], test_config: "libstatssocket_test.xml", - //TODO(b/153588990): Remove when the build system properly separates + //TODO(b/153588990): Remove when the build system properly separates. //32bit and 64bit architectures. compile_multilib: "both", multilib: { diff --git a/libstats/socket/benchmark/main.cpp b/libstats/socket/benchmark/main.cpp deleted file mode 100644 index 5ebdf6e9a..000000000 --- a/libstats/socket/benchmark/main.cpp +++ /dev/null @@ -1,19 +0,0 @@ -/* - * Copyright (C) 2019 The Android Open Source Project - * - * 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 - -BENCHMARK_MAIN(); diff --git a/libstats/socket/benchmark/stats_event_benchmark.cpp b/libstats/socket/benchmark/stats_event_benchmark.cpp deleted file mode 100644 index 3fc6e551f..000000000 --- a/libstats/socket/benchmark/stats_event_benchmark.cpp +++ /dev/null @@ -1,54 +0,0 @@ -/* - * Copyright (C) 2019 The Android Open Source Project - * - * 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 "benchmark/benchmark.h" -#include "stats_event.h" - -static AStatsEvent* constructStatsEvent() { - AStatsEvent* event = AStatsEvent_obtain(); - AStatsEvent_setAtomId(event, 100); - - // randomly sample atom size - int numElements = rand() % 800; - for (int i = 0; i < numElements; i++) { - AStatsEvent_writeInt32(event, i); - } - - return event; -} - -static void BM_stats_event_truncate_buffer(benchmark::State& state) { - while (state.KeepRunning()) { - AStatsEvent* event = constructStatsEvent(); - AStatsEvent_build(event); - AStatsEvent_write(event); - AStatsEvent_release(event); - } -} - -BENCHMARK(BM_stats_event_truncate_buffer); - -static void BM_stats_event_full_buffer(benchmark::State& state) { - while (state.KeepRunning()) { - AStatsEvent* event = constructStatsEvent(); - AStatsEvent_truncateBuffer(event, false); - AStatsEvent_build(event); - AStatsEvent_write(event); - AStatsEvent_release(event); - } -} - -BENCHMARK(BM_stats_event_full_buffer); diff --git a/libstats/socket/include/stats_event.h b/libstats/socket/include/stats_event.h index 601a1815a..3d3baf9cf 100644 --- a/libstats/socket/include/stats_event.h +++ b/libstats/socket/include/stats_event.h @@ -157,9 +157,6 @@ uint32_t AStatsEvent_getAtomId(AStatsEvent* event); uint8_t* AStatsEvent_getBuffer(AStatsEvent* event, size_t* size); uint32_t AStatsEvent_getErrors(AStatsEvent* event); -// exposed for benchmarking only -void AStatsEvent_truncateBuffer(struct AStatsEvent* event, bool truncate); - #ifdef __cplusplus } #endif // __CPLUSPLUS diff --git a/libstats/socket/stats_event.c b/libstats/socket/stats_event.c index a94b3a1ed..f3e808756 100644 --- a/libstats/socket/stats_event.c +++ b/libstats/socket/stats_event.c @@ -76,7 +76,6 @@ struct AStatsEvent { uint32_t numElements; uint32_t atomId; uint32_t errors; - bool truncate; bool built; size_t bufSize; }; @@ -95,7 +94,6 @@ AStatsEvent* AStatsEvent_obtain() { event->numElements = 0; event->atomId = 0; event->errors = 0; - event->truncate = true; // truncate for both pulled and pushed atoms event->built = false; event->bufSize = MAX_PUSH_EVENT_PAYLOAD; event->buf = (uint8_t*)calloc(event->bufSize, 1); @@ -318,10 +316,6 @@ uint32_t AStatsEvent_getErrors(AStatsEvent* event) { return event->errors; } -void AStatsEvent_truncateBuffer(AStatsEvent* event, bool truncate) { - event->truncate = truncate; -} - static void build_internal(AStatsEvent* event, const bool push) { if (event->numElements > MAX_BYTE_VALUE) event->errors |= ERROR_TOO_MANY_FIELDS; if (0 == event->atomId) event->errors |= ERROR_NO_ATOM_ID; @@ -341,13 +335,6 @@ static void build_internal(AStatsEvent* event, const bool push) { } event->buf[POS_NUM_ELEMENTS] = event->numElements; - - // Truncate the buffer to the appropriate length in order to limit our - // memory usage. - if (event->truncate) { - event->buf = (uint8_t*)realloc(event->buf, event->numBytesWritten); - event->bufSize = event->numBytesWritten; - } } void AStatsEvent_build(AStatsEvent* event) { diff --git a/libstats/socket/tests/stats_event_test.cpp b/libstats/socket/tests/stats_event_test.cpp index cc2552119..9a1fac89f 100644 --- a/libstats/socket/tests/stats_event_test.cpp +++ b/libstats/socket/tests/stats_event_test.cpp @@ -18,7 +18,7 @@ #include #include -// Keep in sync stats_event.c. Consider moving to separate header file to avoid duplication. +// Keep in sync with stats_event.c. Consider moving to separate header file to avoid duplication. /* ERRORS */ #define ERROR_NO_TIMESTAMP 0x1 #define ERROR_NO_ATOM_ID 0x2