From 79dd3eaa878d35c7d586e4d2929356b3b0c1a79c Mon Sep 17 00:00:00 2001 From: Ruchir Rastogi Date: Thu, 12 Dec 2019 17:16:59 -0800 Subject: [PATCH] Improve stats_event memory usage We now truncate the buffer to the appropriate length when clients call stats_event_build(). Benchmarking tests indicate that truncating the buffer to the appropriate length increases the cost clients pay to write to the socket by 2%. This is negligible enough that I decided to truncate the buffer for both pushed and pulled atoms in order to simplify the API. Test: m libstatssocket Test: bit libstatssocket_benchmark:* Bug: 144126231 Change-Id: I35dec748ff87c0821d0d06779a406997e6e64966 Merged-In: Ife976bb383ecff8de5064730692a95e2a3a82c9d --- libstats/socket/Android.bp | 20 +++++++ libstats/socket/benchmark/main.cpp | 19 +++++++ .../benchmark/stats_event_benchmark.cpp | 53 +++++++++++++++++++ libstats/socket/include/stats_event.h | 3 ++ libstats/socket/stats_event.c | 19 ++++--- 5 files changed, 108 insertions(+), 6 deletions(-) create mode 100644 libstats/socket/benchmark/main.cpp create mode 100644 libstats/socket/benchmark/stats_event_benchmark.cpp diff --git a/libstats/socket/Android.bp b/libstats/socket/Android.bp index bd3d9ae6f..94c405dea 100644 --- a/libstats/socket/Android.bp +++ b/libstats/socket/Android.bp @@ -47,3 +47,23 @@ cc_library_headers { export_include_dirs: ["include"], host_supported: true, } + +cc_benchmark { + name: "libstatssocket_benchmark", + srcs: [ + "benchmark/main.cpp", + "benchmark/stats_event_benchmark.cpp", + ], + cflags: [ + "-Wall", + "-Werror", + ], + static_libs: [ + "libstatssocket", + ], + shared_libs: [ + "libcutils", + "liblog", + "libgtest_prod", + ], +} diff --git a/libstats/socket/benchmark/main.cpp b/libstats/socket/benchmark/main.cpp new file mode 100644 index 000000000..5ebdf6e9a --- /dev/null +++ b/libstats/socket/benchmark/main.cpp @@ -0,0 +1,19 @@ +/* + * 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 new file mode 100644 index 000000000..b487c4d4b --- /dev/null +++ b/libstats/socket/benchmark/stats_event_benchmark.cpp @@ -0,0 +1,53 @@ +/* + * 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 struct stats_event* constructStatsEvent() { + struct stats_event* event = stats_event_obtain(); + stats_event_set_atom_id(event, 100); + + // randomly sample atom size + for (int i = 0; i < rand() % 800; i++) { + stats_event_write_int32(event, i); + } + + return event; +} + +static void BM_stats_event_truncate_buffer(benchmark::State& state) { + while (state.KeepRunning()) { + struct stats_event* event = constructStatsEvent(); + stats_event_build(event); + stats_event_write(event); + stats_event_release(event); + } +} + +BENCHMARK(BM_stats_event_truncate_buffer); + +static void BM_stats_event_full_buffer(benchmark::State& state) { + while (state.KeepRunning()) { + struct stats_event* event = constructStatsEvent(); + stats_event_truncate_buffer(event, false); + stats_event_build(event); + stats_event_write(event); + stats_event_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 e7117d2a0..080e957b2 100644 --- a/libstats/socket/include/stats_event.h +++ b/libstats/socket/include/stats_event.h @@ -154,6 +154,9 @@ struct stats_event_api_table { uint32_t (*get_errors)(struct stats_event*); }; +// exposed for benchmarking only +void stats_event_truncate_buffer(struct stats_event* event, bool truncate); + #ifdef __cplusplus } #endif // __CPLUSPLUS diff --git a/libstats/socket/stats_event.c b/libstats/socket/stats_event.c index 409843410..551b392df 100644 --- a/libstats/socket/stats_event.c +++ b/libstats/socket/stats_event.c @@ -20,7 +20,6 @@ #include #include "stats_buffer_writer.h" -#define STATS_EVENT_TAG 1937006964 #define LOGGER_ENTRY_MAX_PAYLOAD 4068 // Max payload size is 4 bytes less as 4 bytes are reserved for stats_eventTag. // See android_util_Stats_Log.cpp @@ -39,13 +38,13 @@ // The stats_event struct holds the serialized encoding of an event // within a buf. Also includes other required fields. struct stats_event { - uint8_t buf[MAX_EVENT_PAYLOAD]; + uint8_t* buf; size_t lastFieldPos; // location of last field within the buf size_t size; // number of valid bytes within buffer uint32_t numElements; uint32_t atomId; uint32_t errors; - uint32_t tag; + bool truncate; bool built; }; @@ -58,12 +57,11 @@ static int64_t get_elapsed_realtime_ns() { struct stats_event* stats_event_obtain() { struct stats_event* event = malloc(sizeof(struct stats_event)); - - memset(event->buf, 0, MAX_EVENT_PAYLOAD); + event->buf = (uint8_t*)calloc(MAX_EVENT_PAYLOAD, 1); event->buf[0] = OBJECT_TYPE; event->atomId = 0; event->errors = 0; - event->tag = STATS_EVENT_TAG; + event->truncate = true; // truncate for both pulled and pushed atoms event->built = false; // place the timestamp @@ -79,6 +77,7 @@ struct stats_event* stats_event_obtain() { } void stats_event_release(struct stats_event* event) { + free(event->buf); free(event); } @@ -297,6 +296,10 @@ uint32_t stats_event_get_errors(struct stats_event* event) { return event->errors; } +void stats_event_truncate_buffer(struct stats_event* event, bool truncate) { + event->truncate = truncate; +} + void stats_event_build(struct stats_event* event) { if (event->built) return; @@ -317,6 +320,10 @@ void stats_event_build(struct stats_event* event) { event->size = POS_FIRST_FIELD + sizeof(uint8_t) + sizeof(uint32_t); } + // 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->size); + event->built = true; }