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
diff --git a/libstats/socket/Android.bp b/libstats/socket/Android.bp
index 2bf0261..bf79ea2 100644
--- a/libstats/socket/Android.bp
+++ b/libstats/socket/Android.bp
@@ -89,25 +89,6 @@
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 @@
],
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 5ebdf6e..0000000
--- 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/benchmark.h>
-
-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 3fc6e55..0000000
--- 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 601a181..3d3baf9 100644
--- a/libstats/socket/include/stats_event.h
+++ b/libstats/socket/include/stats_event.h
@@ -157,9 +157,6 @@
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 a94b3a1..f3e8087 100644
--- a/libstats/socket/stats_event.c
+++ b/libstats/socket/stats_event.c
@@ -76,7 +76,6 @@
uint32_t numElements;
uint32_t atomId;
uint32_t errors;
- bool truncate;
bool built;
size_t bufSize;
};
@@ -95,7 +94,6 @@
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 @@
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 @@
}
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 cc25521..9a1fac8 100644
--- a/libstats/socket/tests/stats_event_test.cpp
+++ b/libstats/socket/tests/stats_event_test.cpp
@@ -18,7 +18,7 @@
#include <gtest/gtest.h>
#include <utils/SystemClock.h>
-// 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