From 65c411433bebe2833825651b2ad2eedb418172ee Mon Sep 17 00:00:00 2001 From: Trey Moen <50057480+greatgitsby@users.noreply.github.com> Date: Mon, 7 Sep 2026 01:22:23 -0700 Subject: [PATCH] cabana: reduce downloader startup and dense chart rendering overhead (#38793) * cabana: reduce downloader startup and dense chart rendering overhead * cabana: remove downloader spawn tests --- openpilot/tools/cabana/tests/test_cabana.cc | 25 ++++++ openpilot/tools/cabana/ui/chart/chart.cc | 9 +- openpilot/tools/cabana/ui/chart/downsample.h | 39 +++++++++ openpilot/tools/lib/file_downloader.py | 10 +-- openpilot/tools/replay/py_downloader.cc | 87 +++++++++++++------- 5 files changed, 133 insertions(+), 37 deletions(-) create mode 100644 openpilot/tools/cabana/ui/chart/downsample.h diff --git a/openpilot/tools/cabana/tests/test_cabana.cc b/openpilot/tools/cabana/tests/test_cabana.cc index 3523a47e08..e1c98fcc8e 100644 --- a/openpilot/tools/cabana/tests/test_cabana.cc +++ b/openpilot/tools/cabana/tests/test_cabana.cc @@ -10,6 +10,7 @@ #include "tools/cabana/dbc/dbcmanager.h" #include "tools/cabana/routes.h" #include "tools/cabana/ui/qtstate.h" +#include "tools/cabana/ui/chart/downsample.h" #include "tools/cabana/utils/strings.h" const std::string TEST_RLOG_URL = "https://commadataci.blob.core.windows.net/openpilotci/0c94aa1e1296d7c6/2021-05-05--19-48-37/0/rlog.bz2"; @@ -353,7 +354,31 @@ void test_qt_state_blobs() { REQUIRE(!qtstate::parseQtHeaderState(fromHex("000000fe00000000000000010000000000000000010000000000000000")).has_value()); } +void test_pixel_envelope() { + struct Point { + double x, y; + Point(double x, double y) : x(x), y(y) {} + }; + std::vector points; + for (int i = 0; i < 1000; ++i) points.emplace_back(i * 0.001, i == 203 ? 99 : i == 201 ? -99 : 0); + const auto result = chart::pixelEnvelope(points.begin(), points.end(), 0, 1, 10); + REQUIRE(result.size() <= 40); + REQUIRE(result.front().x == points.front().x); + REQUIRE(result.back().x == points.back().x); + REQUIRE(std::is_sorted(result.begin(), result.end(), [](const auto &a, const auto &b) { return a.x < b.x; })); + REQUIRE(std::any_of(result.begin(), result.end(), [](const auto &p) { return p.x == .201 && p.y == -99; })); + REQUIRE(std::any_of(result.begin(), result.end(), [](const auto &p) { return p.x == .203 && p.y == 99; })); + const std::vector step{{-1, 0}, {0, 0}, {0, 10}, {0, -10}, {0, 0}, {2, 0}}; + const auto edge = chart::pixelEnvelope(step.begin(), step.end(), 0, 1, 2); + REQUIRE(edge.front().x == -1); + REQUIRE(edge.back().x == 2); + REQUIRE(std::any_of(edge.begin(), edge.end(), [](const auto &p) { return p.y == 10; })); + REQUIRE(std::any_of(edge.begin(), edge.end(), [](const auto &p) { return p.y == -10; })); + REQUIRE(chart::pixelEnvelope(points.begin(), points.begin(), 0, 1, 10).empty()); +} + void test_cabana_core() { + test_pixel_envelope(); test_format_seconds(); test_to_hex(); test_signal_tooltip(); diff --git a/openpilot/tools/cabana/ui/chart/chart.cc b/openpilot/tools/cabana/ui/chart/chart.cc index ea54fc819b..399304186e 100644 --- a/openpilot/tools/cabana/ui/chart/chart.cc +++ b/openpilot/tools/cabana/ui/chart/chart.cc @@ -11,6 +11,7 @@ #include "tools/cabana/core/settings.h" #include "tools/cabana/settings.h" #include "tools/cabana/ui/chart/chartswidget.h" +#include "tools/cabana/ui/chart/downsample.h" #include "tools/cabana/ui/icons.h" #include "tools/cabana/ui/util.h" #include "tools/cabana/utils/strings.h" @@ -665,7 +666,13 @@ void ChartView::drawSeries() { if (begin == end) continue; spec.LineWeight = 2; - ImPlot::PlotLine(label.c_str(), &begin->x, &begin->y, end - begin, spec); + const int pixels = std::max(1, (int)layout_.plot_area.GetWidth()); + if (end - begin > pixels * 4) { + const auto envelope = chart::pixelEnvelope(begin, end, x_min_, x_max_, pixels); + ImPlot::PlotLine(label.c_str(), &envelope.front().x, &envelope.front().y, envelope.size(), spec); + } else { + ImPlot::PlotLine(label.c_str(), &begin->x, &begin->y, end - begin, spec); + } // show points when zoomed in enough if ((num_points == 1 || pixels_per_point > 20) && first != last) { diff --git a/openpilot/tools/cabana/ui/chart/downsample.h b/openpilot/tools/cabana/ui/chart/downsample.h new file mode 100644 index 0000000000..e523125b62 --- /dev/null +++ b/openpilot/tools/cabana/ui/chart/downsample.h @@ -0,0 +1,39 @@ +#pragma once + +#include +#include +#include +#include +#include + +namespace chart { + +// Keep endpoints and both extrema in time order in each pixel column. Only rendering +// uses this envelope; calculations, cursor values and exports retain every sample. +template +auto pixelEnvelope(Iterator begin, Iterator end, double min, double max, int pixels) { + using Point = typename std::iterator_traits::value_type; + std::vector result; + if (begin == end || pixels <= 0 || max <= min) return result; + result.reserve(std::min(end - begin, (size_t)pixels * 4)); + auto column = [&](const auto &p) { + return (int)std::clamp((p.x - min) / (max - min) * pixels, 0.0, double(pixels - 1)); + }; + while (begin != end) { + auto last = begin, low = begin, high = begin, next = begin + 1; + const int bucket = column(*begin); + while (next != end && column(*next) == bucket) { + if (next->y < low->y) low = next; + if (next->y > high->y) high = next; + last = next++; + } + std::array selected{begin, low, high, last}; + std::sort(selected.begin(), selected.end()); + auto unique_end = std::unique(selected.begin(), selected.end()); + for (auto it = selected.begin(); it != unique_end; ++it) result.push_back(**it); + begin = next; + } + return result; +} + +} // namespace chart diff --git a/openpilot/tools/lib/file_downloader.py b/openpilot/tools/lib/file_downloader.py index efb06095be..fd517d7ada 100755 --- a/openpilot/tools/lib/file_downloader.py +++ b/openpilot/tools/lib/file_downloader.py @@ -19,16 +19,14 @@ import shutil import sys import tempfile -import zstandard as zstd - from openpilot.common.hardware.hw import Paths -from openpilot.tools.lib.api import CommaApi, UnauthorizedError, APIError -from openpilot.tools.lib.auth_config import get_token -from openpilot.tools.lib.url_file import URLFile def api_call(func): """Run an API call, outputting JSON result or error to stdout.""" + from openpilot.tools.lib.api import CommaApi, UnauthorizedError, APIError + from openpilot.tools.lib.auth_config import get_token + try: result = func(CommaApi(get_token())) json.dump(result, sys.stdout) @@ -62,6 +60,7 @@ def make_decompressor(compression): if compression == 'bz2': return bz2.BZ2Decompressor() if compression == 'zst': + import zstandard as zstd return zstd.ZstdDecompressor().decompressobj() raise ValueError(f"Unsupported compression type: {compression}") @@ -126,6 +125,7 @@ def cmd_download(args): try: # Stream the file in a single HTTP request instead of making # a separate Range request per chunk (which was very slow). + from openpilot.tools.lib.url_file import URLFile pool = URLFile.pool_manager() r = pool.request("GET", url, preload_content=False) if r.status not in (200, 206): diff --git a/openpilot/tools/replay/py_downloader.cc b/openpilot/tools/replay/py_downloader.cc index db2b2be127..a7ab5baa91 100644 --- a/openpilot/tools/replay/py_downloader.cc +++ b/openpilot/tools/replay/py_downloader.cc @@ -5,6 +5,10 @@ #include #include #include +#include +#ifdef __APPLE__ +#include +#endif #include #include #include @@ -27,7 +31,7 @@ void reportProgress(const char *line) { // Run a Python command and capture stdout. Stderr is scanned for PROGRESS lines and otherwise passed // through to the parent's stderr. Returns stdout content. If abort is signaled, kills the child process. std::string runPython(const std::vector &args, std::atomic *abort = nullptr) { - // Build argv for execvp + // Build argv for the downloader module std::vector argv; argv.push_back("python3"); argv.push_back("-m"); @@ -37,50 +41,71 @@ std::string runPython(const std::vector &args, std::atomic *a } argv.push_back(nullptr); + auto open_pipe = [](int (&fds)[2]) { +#ifdef __linux__ + return pipe2(fds, O_CLOEXEC); +#else + if (pipe(fds) != 0) return -1; + if (fcntl(fds[0], F_SETFD, FD_CLOEXEC) == 0 && fcntl(fds[1], F_SETFD, FD_CLOEXEC) == 0) return 0; + close(fds[0]); close(fds[1]); + return -1; +#endif + }; int stdout_pipe[2], stderr_pipe[2]; - if (pipe(stdout_pipe) != 0) { + if (open_pipe(stdout_pipe) != 0) { rWarning("py_downloader: pipe() failed"); return {}; } - if (pipe(stderr_pipe) != 0) { + if (open_pipe(stderr_pipe) != 0) { rWarning("py_downloader: pipe() failed"); close(stdout_pipe[0]); close(stdout_pipe[1]); return {}; } - pid_t pid = fork(); - if (pid < 0) { - rWarning("py_downloader: fork() failed"); + // Avoid copying the large replay address space and running atfork handlers on + // every segment download: both can stall rendering even from a worker thread. + std::vector environment; +#ifdef __APPLE__ + char **parent_environment = *_NSGetEnviron(); +#else + char **parent_environment = environ; +#endif + for (char **entry = parent_environment; *entry; ++entry) { + if (strncmp(*entry, "OPENPILOT_PREFIX=", 17) != 0) environment.emplace_back(*entry); + } + std::vector envp; + for (auto &entry : environment) envp.push_back(entry.data()); + envp.push_back(nullptr); + + posix_spawn_file_actions_t actions; + posix_spawnattr_t attributes; + int error = posix_spawn_file_actions_init(&actions); + const bool actions_initialized = error == 0; + if (!error) error = posix_spawnattr_init(&attributes); + const bool attributes_initialized = error == 0; + if (!error) error = posix_spawn_file_actions_addopen(&actions, STDIN_FILENO, "/dev/null", O_RDONLY, 0); + if (!error) error = posix_spawn_file_actions_adddup2(&actions, stdout_pipe[1], STDOUT_FILENO); + if (!error) error = posix_spawn_file_actions_adddup2(&actions, stderr_pipe[1], STDERR_FILENO); + for (int fd : {stdout_pipe[0], stdout_pipe[1], stderr_pipe[0], stderr_pipe[1]}) { + if (!error) error = posix_spawn_file_actions_addclose(&actions, fd); + } +#ifdef POSIX_SPAWN_SETSID + if (!error) error = posix_spawnattr_setflags(&attributes, POSIX_SPAWN_SETSID); +#else + if (!error) error = posix_spawnattr_setpgroup(&attributes, 0); + if (!error) error = posix_spawnattr_setflags(&attributes, POSIX_SPAWN_SETPGROUP); +#endif + pid_t pid = -1; + if (!error) error = posix_spawnp(&pid, "python3", &actions, &attributes, const_cast(argv.data()), envp.data()); + if (attributes_initialized) posix_spawnattr_destroy(&attributes); + if (actions_initialized) posix_spawn_file_actions_destroy(&actions); + if (error) { + rWarning("py_downloader: posix_spawnp() failed: %s", strerror(error)); close(stdout_pipe[0]); close(stdout_pipe[1]); close(stderr_pipe[0]); close(stderr_pipe[1]); return {}; } - if (pid == 0) { - // Child process — detach from controlling terminal so Python - // cannot corrupt terminal settings needed by ncurses in the parent. - setsid(); - int devnull = open("/dev/null", O_RDONLY); - if (devnull >= 0) { - dup2(devnull, STDIN_FILENO); - if (devnull > STDERR_FILENO) close(devnull); - } - - // Clear OPENPILOT_PREFIX so the Python process uses default paths - // (e.g. ~/.comma/auth.json). The prefix is only for IPC in the parent. - unsetenv("OPENPILOT_PREFIX"); - - close(stdout_pipe[0]); - dup2(stdout_pipe[1], STDOUT_FILENO); - close(stdout_pipe[1]); - close(stderr_pipe[0]); - dup2(stderr_pipe[1], STDERR_FILENO); - close(stderr_pipe[1]); - - execvp("python3", const_cast(argv.data())); - _exit(127); - } - // Parent process close(stdout_pipe[1]); close(stderr_pipe[1]);