mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-20 18:09:05 +00:00
Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
73fff8d2d5 | ||
|
|
446207ffbc |
@@ -8,6 +8,18 @@ Type=simple
|
|||||||
User=root
|
User=root
|
||||||
WorkingDirectory=__PROJECT_ROOT_DIR__
|
WorkingDirectory=__PROJECT_ROOT_DIR__
|
||||||
Environment=PYTHONDONTWRITEBYTECODE=1
|
Environment=PYTHONDONTWRITEBYTECODE=1
|
||||||
|
# glibc gives each allocating thread its own malloc arena, up to 8 x CPU count,
|
||||||
|
# and an arena that has grown is never handed back to the OS. This process runs
|
||||||
|
# 9 threads on a 3-core Pi, so the ceiling is 24 arenas -- and a rig measured at
|
||||||
|
# 1030 MB resident held 23 large anonymous mappings on 64 MB-aligned addresses,
|
||||||
|
# 920 MB of them, while the live data it was actually holding (widest scroll
|
||||||
|
# strip seen: 35,746 x 64) accounts for roughly 15 MB. That gap is arena bloat,
|
||||||
|
# not leaked objects: RSS was flat across repeated sampling, not climbing.
|
||||||
|
#
|
||||||
|
# Capping the arenas trades a little allocator concurrency for a large amount of
|
||||||
|
# resident memory on a device that has neither to spare. 2 is the usual value;
|
||||||
|
# raise it if frame times regress.
|
||||||
|
Environment=MALLOC_ARENA_MAX=2
|
||||||
ExecStart=/usr/bin/python3 __PROJECT_ROOT_DIR__/run.py
|
ExecStart=/usr/bin/python3 __PROJECT_ROOT_DIR__/run.py
|
||||||
# Restart=always, not on-failure: run.py exiting 0 (a clean shutdown path taken
|
# Restart=always, not on-failure: run.py exiting 0 (a clean shutdown path taken
|
||||||
# for a reason that no longer applies, e.g. a config reload) would otherwise leave
|
# for a reason that no longer applies, e.g. a config reload) would otherwise leave
|
||||||
|
|||||||
@@ -0,0 +1,95 @@
|
|||||||
|
"""The display unit must cap glibc's malloc arenas.
|
||||||
|
|
||||||
|
glibc hands each allocating thread its own malloc arena, up to 8 x CPU count,
|
||||||
|
and an arena that has grown is never returned to the OS. This process runs
|
||||||
|
threads for the render loop, the update workers and the background fetchers, so
|
||||||
|
on a 3-core Pi the ceiling is 24 arenas.
|
||||||
|
|
||||||
|
Measured on a live rig, 2.5 hours in:
|
||||||
|
|
||||||
|
RSS 1030 MB
|
||||||
|
Private_Dirty 988 MB
|
||||||
|
anonymous mappings > 10 MB 23 (ceiling is 8 x 3 = 24)
|
||||||
|
largest few 104, 79, 66, 63, 63 MB, on 64 MB-aligned addresses
|
||||||
|
|
||||||
|
against live data that accounts for perhaps 15 MB -- the widest scroll strip
|
||||||
|
observed was 35,746 x 64, about 7 MB as RGB and the same again for its numpy
|
||||||
|
mirror. Repeated sampling showed RSS flat between 990 and 1030 MB rather than
|
||||||
|
climbing, so this is arena bloat rather than a leak: memory Python has freed
|
||||||
|
but glibc is holding per-arena.
|
||||||
|
|
||||||
|
The device had 59 MB free at the time.
|
||||||
|
|
||||||
|
Capping the arena count trades a little allocator concurrency for that resident
|
||||||
|
memory. The render loop is latency-sensitive, so if p99 frame time regresses the
|
||||||
|
right response is to raise this rather than remove it.
|
||||||
|
"""
|
||||||
|
import re
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
UNIT = (Path(__file__).resolve().parent.parent / "systemd" / "ledmatrix.service")
|
||||||
|
|
||||||
|
#: The value the unit is expected to carry. 2 is the usual choice for a
|
||||||
|
#: threaded Python process; 1-4 all keep some of the saving, but only one of
|
||||||
|
#: them is what this project ships.
|
||||||
|
EXPECTED_ARENA_MAX = 2
|
||||||
|
|
||||||
|
|
||||||
|
def _environment(unit_text):
|
||||||
|
return dict(
|
||||||
|
line.split("=", 2)[1:3] if line.count("=") >= 2 else (line.split("=", 1)[1], "")
|
||||||
|
for line in unit_text.splitlines()
|
||||||
|
if line.startswith("Environment=")
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_unit_exists():
|
||||||
|
assert UNIT.is_file(), f"{UNIT} is missing"
|
||||||
|
|
||||||
|
|
||||||
|
def test_malloc_arena_max_is_capped():
|
||||||
|
env = _environment(UNIT.read_text(encoding="utf-8"))
|
||||||
|
assert "MALLOC_ARENA_MAX" in env, (
|
||||||
|
"the display unit does not cap glibc arenas; on a 3-core Pi the default "
|
||||||
|
"ceiling is 24 and a measured rig held 23 of them, 920 MB"
|
||||||
|
)
|
||||||
|
value = int(env["MALLOC_ARENA_MAX"])
|
||||||
|
# Pinned, not a range. A range let a change to 4 -- which hands most of the
|
||||||
|
# saving back -- pass unnoticed, which was the point of the finding that
|
||||||
|
# prompted this. Raising it is a legitimate response to a frame-time
|
||||||
|
# regression, but it should be a visible edit here rather than a silent
|
||||||
|
# drift, so the number lives in one place and changing it shows up in
|
||||||
|
# review.
|
||||||
|
assert value == EXPECTED_ARENA_MAX, (
|
||||||
|
f"MALLOC_ARENA_MAX={value}, expected {EXPECTED_ARENA_MAX}. If this was "
|
||||||
|
"raised deliberately because frame times regressed, update "
|
||||||
|
"EXPECTED_ARENA_MAX here and say so in the commit."
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_reason_is_recorded_next_to_it():
|
||||||
|
"""A bare tuning knob invites removal by whoever meets it next."""
|
||||||
|
text = UNIT.read_text(encoding="utf-8")
|
||||||
|
index = text.index("Environment=MALLOC_ARENA_MAX")
|
||||||
|
preamble = text[:index].splitlines()[-12:]
|
||||||
|
comment = "\n".join(line for line in preamble if line.startswith("#"))
|
||||||
|
assert "arena" in comment.lower(), "no explanation precedes the setting"
|
||||||
|
assert re.search(r"\d", comment), (
|
||||||
|
"the explanation cites no measurement, so a reader cannot tell whether "
|
||||||
|
"it still applies to their hardware"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("unit", ["ledmatrix.service"])
|
||||||
|
def test_the_unit_still_parses_as_ini(unit):
|
||||||
|
"""systemd will refuse a malformed unit, and the panel stays dark."""
|
||||||
|
import configparser
|
||||||
|
|
||||||
|
path = UNIT.parent / unit
|
||||||
|
parser = configparser.ConfigParser(strict=False)
|
||||||
|
# systemd allows repeated keys; ConfigParser needs them merged, not rejected.
|
||||||
|
parser.read_string(path.read_text(encoding="utf-8"))
|
||||||
|
assert parser.has_section("Service")
|
||||||
|
assert parser.has_option("Service", "ExecStart")
|
||||||
@@ -194,46 +194,15 @@ class TestSavePluginConfig:
|
|||||||
|
|
||||||
def test_secret_count_message_counts_top_level_keys(self, env):
|
def test_secret_count_message_counts_top_level_keys(self, env):
|
||||||
# Pinned: the "(N secret field(s))" message counts TOP-LEVEL keys of
|
# Pinned: the "(N secret field(s))" message counts TOP-LEVEL keys of
|
||||||
# the separated secrets dict. Here that is 1: the posted accounts
|
# the separated secrets dict. Here that is 2: the posted accounts
|
||||||
# array, whose item tokens all count as ONE key.
|
# array (all its item tokens count as ONE key) plus the schema's
|
||||||
#
|
# api_key default ("") that merge_with_defaults adds before
|
||||||
# It was 2 before blank secrets were dropped, the second being the
|
# separation.
|
||||||
# schema's api_key default (""), which merge_with_defaults adds to
|
|
||||||
# every save. Counting it was the visible edge of a real bug: that
|
|
||||||
# injected blank was merged over the stored api_key, so saving any
|
|
||||||
# unrelated field destroyed the credential. See
|
|
||||||
# test_an_unrelated_edit_does_not_erase_a_stored_secret.
|
|
||||||
resp = self._save(env, {
|
resp = self._save(env, {
|
||||||
"accounts": [{"name": "a", "token": "t"}],
|
"accounts": [{"name": "a", "token": "t"}],
|
||||||
})
|
})
|
||||||
message = resp.get_json()["message"]
|
message = resp.get_json()["message"]
|
||||||
assert "(1 secret field(s) saved to config_secrets.json)" in message
|
assert "(2 secret field(s) saved to config_secrets.json)" in message
|
||||||
|
|
||||||
def test_an_unrelated_edit_does_not_erase_a_stored_secret(self, env):
|
|
||||||
"""Editing one field must not wipe the plugin's API key.
|
|
||||||
|
|
||||||
The config form renders secrets masked, so the browser posts them
|
|
||||||
back blank; merge_with_defaults injects a blank api_key even when
|
|
||||||
the client omits it entirely. Either way a "" reached the secrets
|
|
||||||
file and deep_merge wrote it over the stored credential.
|
|
||||||
"""
|
|
||||||
assert self._save(env, {"api_key": "REAL-KEY-0123456789",
|
|
||||||
"city": "Austin"}).status_code == 200
|
|
||||||
assert _on_disk(env.secrets_file)[PLUGIN_ID]["api_key"] == \
|
|
||||||
"REAL-KEY-0123456789"
|
|
||||||
|
|
||||||
# the user changes the city; the masked api_key rides along blank
|
|
||||||
assert self._save(env, {"api_key": "", "city": "Dallas"}).status_code == 200
|
|
||||||
|
|
||||||
assert _on_disk(env.secrets_file)[PLUGIN_ID]["api_key"] == \
|
|
||||||
"REAL-KEY-0123456789", "an unrelated edit destroyed the API key"
|
|
||||||
assert env.fresh_load()[PLUGIN_ID]["city"] == "Dallas"
|
|
||||||
|
|
||||||
def test_a_secret_can_still_be_changed(self, env):
|
|
||||||
"""Dropping blanks must not stop a real new value from being saved."""
|
|
||||||
self._save(env, {"api_key": "first-key"})
|
|
||||||
self._save(env, {"api_key": "second-key"})
|
|
||||||
assert _on_disk(env.secrets_file)[PLUGIN_ID]["api_key"] == "second-key"
|
|
||||||
|
|
||||||
def test_resave_replaces_stored_secrets_list_wholesale(self, env):
|
def test_resave_replaces_stored_secrets_list_wholesale(self, env):
|
||||||
# Characterized: api_v3's deep_merge intentionally replaces lists,
|
# Characterized: api_v3's deep_merge intentionally replaces lists,
|
||||||
|
|||||||
@@ -21,8 +21,7 @@ logger = logging.getLogger(__name__)
|
|||||||
# Import new infrastructure
|
# Import new infrastructure
|
||||||
from src.web_interface.api_helpers import success_response, error_response, validate_request_json
|
from src.web_interface.api_helpers import success_response, error_response, validate_request_json
|
||||||
from src.web_interface.errors import ErrorCode
|
from src.web_interface.errors import ErrorCode
|
||||||
from src.web_interface.secret_helpers import (find_secret_fields, remove_empty_secrets,
|
from src.web_interface.secret_helpers import find_secret_fields, separate_secrets
|
||||||
separate_secrets)
|
|
||||||
from src.web_interface.error_handler import describe_exception, redact_text
|
from src.web_interface.error_handler import describe_exception, redact_text
|
||||||
from src.plugin_system.operation_types import OperationType
|
from src.plugin_system.operation_types import OperationType
|
||||||
from src.web_interface.validators import (
|
from src.web_interface.validators import (
|
||||||
@@ -1217,11 +1216,6 @@ def save_main_config():
|
|||||||
|
|
||||||
# Separate secrets from regular config (same logic as save_plugin_config)
|
# Separate secrets from regular config (same logic as save_plugin_config)
|
||||||
regular_config, secrets_config = separate_secrets(plugin_config, secret_fields)
|
regular_config, secrets_config = separate_secrets(plugin_config, secret_fields)
|
||||||
# The config form renders secrets masked, so every save posts
|
|
||||||
# them back blank. Without this the blank is merged over the
|
|
||||||
# stored value and the credential is destroyed by the act of
|
|
||||||
# changing an unrelated setting. A blank means "unchanged".
|
|
||||||
secrets_config = remove_empty_secrets(secrets_config)
|
|
||||||
|
|
||||||
# PRE-PROCESSING: Preserve 'enabled' state if not in regular_config
|
# PRE-PROCESSING: Preserve 'enabled' state if not in regular_config
|
||||||
# This prevents overwriting the enabled state when saving config from a form that doesn't include the toggle
|
# This prevents overwriting the enabled state when saving config from a form that doesn't include the toggle
|
||||||
@@ -5605,11 +5599,6 @@ def save_plugin_config():
|
|||||||
# Separate secrets from regular config (handles nested configs and
|
# Separate secrets from regular config (handles nested configs and
|
||||||
# array-item secrets — see src/web_interface/secret_helpers.py)
|
# array-item secrets — see src/web_interface/secret_helpers.py)
|
||||||
regular_config, secrets_config = separate_secrets(plugin_config, secret_fields)
|
regular_config, secrets_config = separate_secrets(plugin_config, secret_fields)
|
||||||
# The config form renders secrets masked, so every save posts
|
|
||||||
# them back blank. Without this the blank is merged over the
|
|
||||||
# stored value and the credential is destroyed by the act of
|
|
||||||
# changing an unrelated setting. A blank means "unchanged".
|
|
||||||
secrets_config = remove_empty_secrets(secrets_config)
|
|
||||||
|
|
||||||
# Get current configs
|
# Get current configs
|
||||||
current_config = api_v3.config_manager.load_config()
|
current_config = api_v3.config_manager.load_config()
|
||||||
|
|||||||
Reference in New Issue
Block a user