Optimizes niceness and cpulimit usage
This commit is contained in:
@@ -14,6 +14,7 @@
|
||||
- `FileProperties` now uses one cached `ffprobe -show_format -show_streams -of json` call per source file, and the combined payload was confirmed against the Dragonball asset to satisfy both previous probe call sites fully.
|
||||
- Database startup now bootstraps schema only when required tables are actually missing, while version enforcement still runs on ordinary DB-backed context creation.
|
||||
- Helper filename and rich-text utilities now use compiled raw regexes plus translate-based filename filtering, with unit coverage for TMDB suffix rewriting and Rich color stripping.
|
||||
- Process resource limiting now has explicit disabled/default states in the CLI and requirements, and combined CPU-plus-niceness wrapping now executes as `cpulimit -- nice -n ... <command>` instead of a less explicit prefix chain.
|
||||
- FFX logger setup now reuses named handlers, and fallback logger access no longer mutates handlers in ordinary constructors and helpers.
|
||||
- The process wrapper now uses `subprocess.run(...)` with centralized command formatting plus stable timeout and missing-command error mapping.
|
||||
- Active ORM controllers now use single-query accessors instead of paired `count()` plus `first()` lookups.
|
||||
@@ -87,15 +88,6 @@
|
||||
- Faster contributor onboarding.
|
||||
- Easier CI adoption later.
|
||||
|
||||
7. Process resource limiting semantics could be clearer
|
||||
- [`src/ffx/process.py`](/home/osgw/.local/src/codex/ffx/src/ffx/process.py) prepends `nice` and `cpulimit` directly when values are set.
|
||||
- Optimization:
|
||||
- Validate and document effective behavior for combined `nice` + `cpulimit`.
|
||||
- Consider explicit no-limit vs configured-limit states in the CLI and requirements.
|
||||
- Expected value:
|
||||
- Fewer surprises in production-like runs.
|
||||
- Easier support for user-reported performance behavior.
|
||||
|
||||
## Open
|
||||
|
||||
- Should optimization work focus first on operator-perceived latency, internal maintainability, or correctness-risk cleanup that also has performance upside?
|
||||
|
||||
@@ -42,7 +42,7 @@
|
||||
- SQLite via SQLAlchemy ORM, with schema rooted in shows, patterns, tracks, media tags, track tags, shifted seasons, and generic properties.
|
||||
- A configuration JSON file supplies optional path, metadata-filtering, and filename-template settings.
|
||||
- Integration adapters:
|
||||
- Process execution wrapper for `ffmpeg`, `ffprobe`, `nice`, and `cpulimit`.
|
||||
- Process execution wrapper for `ffmpeg`, `ffprobe`, `nice`, and `cpulimit`, with explicit disabled states for niceness and CPU limiting and a combined `cpulimit -- nice -n ... <command>` execution shape when both limits are configured.
|
||||
- HTTP adapter for TMDB via `requests`.
|
||||
|
||||
## Data And Interface Notes
|
||||
|
||||
@@ -62,6 +62,10 @@
|
||||
- The system shall support optional TMDB lookups to resolve show names, years, and episode titles when a show ID, season, and episode are available.
|
||||
- The system shall generate output filenames from show metadata, season and episode indices, and episode names using the configured filename template.
|
||||
- The system shall allow CLI overrides for stream languages, stream titles, default and forced tracks, stream order, TMDB show and episode data, output directory, label prefix, and processing resource limits.
|
||||
- Processing resource limit rules:
|
||||
- `--nice` shall accept niceness values from `-20` through `19`; omitting the option shall disable niceness adjustment.
|
||||
- `--cpu` shall accept CPU limit values from `1` through `99`; omitting the option shall disable CPU limiting.
|
||||
- When both limits are configured, the process wrapper shall execute the target command through `cpulimit` around a `nice -n ...` invocation so both limits apply to the launched media command.
|
||||
- The system shall support extracting streams into separate files via `unmux` and reporting suggested crop parameters via `cropdetect`.
|
||||
- The system shall handle invalid input and system failures gracefully by logging warnings or raising `click` errors for missing files, invalid media, missing TMDB credentials, incompatible database versions, and ambiguous track dispositions when prompting is disabled.
|
||||
|
||||
|
||||
@@ -32,6 +32,24 @@ if TYPE_CHECKING:
|
||||
LIGHTWEIGHT_COMMANDS = {None, 'version', 'help', 'configure_workstation', 'upgrade'}
|
||||
|
||||
|
||||
def normalizeNicenessOption(ctx, param, value):
|
||||
from ffx.process import normalizeNiceness
|
||||
|
||||
try:
|
||||
return normalizeNiceness(value)
|
||||
except ValueError as ex:
|
||||
raise click.BadParameter(str(ex)) from ex
|
||||
|
||||
|
||||
def normalizeCpuOption(ctx, param, value):
|
||||
from ffx.process import normalizeCpuPercent
|
||||
|
||||
try:
|
||||
return normalizeCpuPercent(value)
|
||||
except ValueError as ex:
|
||||
raise click.BadParameter(str(ex)) from ex
|
||||
|
||||
|
||||
|
||||
@click.group()
|
||||
@click.pass_context
|
||||
@@ -194,6 +212,7 @@ def upgrade(ctx, branch):
|
||||
|
||||
commandSequences += [
|
||||
['git', 'pull'],
|
||||
[bundlePipPath, 'install', '--upgrade', 'pip', 'setuptools', 'wheel'],
|
||||
[bundlePipPath, 'install', '--editable', '.'],
|
||||
]
|
||||
|
||||
@@ -257,8 +276,22 @@ def getUnmuxSequence(trackDescriptor: TrackDescriptor, sourcePath, targetPrefix,
|
||||
@click.option('-l', '--label', type=str, default='', help='Label to be used as filename prefix')
|
||||
@click.option("-o", "--output-directory", type=str, default='')
|
||||
@click.option("-s", "--subtitles-only", is_flag=True, default=False)
|
||||
@click.option('--nice', type=int, default=99, help='Niceness of started processes')
|
||||
@click.option('--cpu', type=int, default=0, help='Limit CPU for started processes to percent')
|
||||
@click.option(
|
||||
'--nice',
|
||||
type=int,
|
||||
default=None,
|
||||
callback=normalizeNicenessOption,
|
||||
show_default='disabled',
|
||||
help='Adjust niceness of started processes (-20..19). Omit to disable; 99 also disables.',
|
||||
)
|
||||
@click.option(
|
||||
'--cpu',
|
||||
type=int,
|
||||
default=None,
|
||||
callback=normalizeCpuOption,
|
||||
show_default='disabled',
|
||||
help='Limit CPU percent of started processes (1..99). Omit to disable; 0 also disables.',
|
||||
)
|
||||
def unmux(ctx,
|
||||
paths,
|
||||
label,
|
||||
@@ -334,8 +367,22 @@ def unmux(ctx,
|
||||
@click.pass_context
|
||||
|
||||
@click.argument('paths', nargs=-1)
|
||||
@click.option('--nice', type=int, default=99, help='Niceness of started processes')
|
||||
@click.option('--cpu', type=int, default=0, help='Limit CPU for started processes to percent')
|
||||
@click.option(
|
||||
'--nice',
|
||||
type=int,
|
||||
default=None,
|
||||
callback=normalizeNicenessOption,
|
||||
show_default='disabled',
|
||||
help='Adjust niceness of started processes (-20..19). Omit to disable; 99 also disables.',
|
||||
)
|
||||
@click.option(
|
||||
'--cpu',
|
||||
type=int,
|
||||
default=None,
|
||||
callback=normalizeCpuOption,
|
||||
show_default='disabled',
|
||||
help='Limit CPU percent of started processes (1..99). Omit to disable; 0 also disables.',
|
||||
)
|
||||
def cropdetect(ctx,
|
||||
paths,
|
||||
nice,
|
||||
@@ -479,8 +526,22 @@ def checkUniqueDispositions(context, mediaDescriptor: MediaDescriptor):
|
||||
@click.option("--no-signature", is_flag=True, default=False)
|
||||
@click.option("--keep-mkvmerge-metadata", is_flag=True, default=False)
|
||||
|
||||
@click.option('--nice', type=int, default=99, help='Niceness of started processes')
|
||||
@click.option('--cpu', type=int, default=0, help='Limit CPU for started processes to percent')
|
||||
@click.option(
|
||||
'--nice',
|
||||
type=int,
|
||||
default=None,
|
||||
callback=normalizeNicenessOption,
|
||||
show_default='disabled',
|
||||
help='Adjust niceness of started processes (-20..19). Omit to disable; 99 also disables.',
|
||||
)
|
||||
@click.option(
|
||||
'--cpu',
|
||||
type=int,
|
||||
default=None,
|
||||
callback=normalizeCpuOption,
|
||||
show_default='disabled',
|
||||
help='Limit CPU percent of started processes (1..99). Omit to disable; 0 also disables.',
|
||||
)
|
||||
|
||||
@click.option('--rename-only', is_flag=True, default=False, help='Only renaming, no recoding')
|
||||
|
||||
|
||||
@@ -6,29 +6,72 @@ from .logging_utils import get_ffx_logger
|
||||
|
||||
COMMAND_TIMED_OUT_RETURN_CODE = 124
|
||||
COMMAND_NOT_FOUND_RETURN_CODE = 127
|
||||
MIN_NICENESS = -20
|
||||
MAX_NICENESS = 19
|
||||
DISABLED_NICENESS_SENTINEL = 99
|
||||
MIN_CPU_PERCENT = 1
|
||||
MAX_CPU_PERCENT = 99
|
||||
DISABLED_CPU_PERCENT_SENTINEL = 0
|
||||
|
||||
|
||||
def formatCommandSequence(commandSequence: Iterable[str]) -> str:
|
||||
return shlex.join([str(token) for token in commandSequence])
|
||||
|
||||
|
||||
def normalizeNiceness(niceness) -> int | None:
|
||||
if niceness is None:
|
||||
return None
|
||||
|
||||
niceness = int(niceness)
|
||||
if niceness == DISABLED_NICENESS_SENTINEL:
|
||||
return None
|
||||
|
||||
if niceness < MIN_NICENESS or niceness > MAX_NICENESS:
|
||||
raise ValueError(
|
||||
f"Niceness must be between {MIN_NICENESS} and {MAX_NICENESS}, "
|
||||
+ f"or {DISABLED_NICENESS_SENTINEL} to disable."
|
||||
)
|
||||
|
||||
return niceness
|
||||
|
||||
|
||||
def normalizeCpuPercent(cpuPercent) -> int | None:
|
||||
if cpuPercent is None:
|
||||
return None
|
||||
|
||||
cpuPercent = int(cpuPercent)
|
||||
if cpuPercent == DISABLED_CPU_PERCENT_SENTINEL:
|
||||
return None
|
||||
|
||||
if cpuPercent < MIN_CPU_PERCENT or cpuPercent > MAX_CPU_PERCENT:
|
||||
raise ValueError(
|
||||
f"CPU limit must be between {MIN_CPU_PERCENT} and {MAX_CPU_PERCENT}, "
|
||||
+ f"or {DISABLED_CPU_PERCENT_SENTINEL} to disable."
|
||||
)
|
||||
|
||||
return cpuPercent
|
||||
|
||||
|
||||
def getWrappedCommandSequence(commandSequence: List[str], context: dict = None) -> List[str]:
|
||||
"""
|
||||
niceness -20 bis +19
|
||||
cpu_percent: 1 bis 99
|
||||
niceness: -20 to 19, disabled when unset
|
||||
cpu_percent: 1 to 99, disabled when unset
|
||||
|
||||
When both limits are configured, cpulimit wraps a nice-adjusted command:
|
||||
cpulimit -l <cpu> -- nice -n <niceness> <command>
|
||||
"""
|
||||
|
||||
niceSequence = []
|
||||
resourceLimits = (context or {}).get('resource_limits', {})
|
||||
niceness = normalizeNiceness(resourceLimits.get('niceness'))
|
||||
cpu_percent = normalizeCpuPercent(resourceLimits.get('cpu_percent'))
|
||||
wrappedCommandSequence = [str(token) for token in commandSequence]
|
||||
|
||||
niceness = int((context or {}).get('resource_limits', {}).get('niceness', 99))
|
||||
cpu_percent = int((context or {}).get('resource_limits', {}).get('cpu_percent', 0))
|
||||
if niceness is not None:
|
||||
wrappedCommandSequence = ['nice', '-n', str(niceness)] + wrappedCommandSequence
|
||||
if cpu_percent is not None:
|
||||
wrappedCommandSequence = ['cpulimit', '-l', str(cpu_percent), '--'] + wrappedCommandSequence
|
||||
|
||||
if niceness >= -20 and niceness <= 19:
|
||||
niceSequence += ['nice', '-n', str(niceness)]
|
||||
if cpu_percent >= 1:
|
||||
niceSequence += ['cpulimit', '-l', str(cpu_percent), '--']
|
||||
|
||||
return niceSequence + [str(token) for token in commandSequence]
|
||||
return wrappedCommandSequence
|
||||
|
||||
|
||||
def getProcessTimeoutSeconds(context: dict = None, timeoutSeconds: float = None):
|
||||
|
||||
@@ -87,6 +87,7 @@ class UpgradeCommandTests(unittest.TestCase):
|
||||
['git', 'reset', '--hard', 'HEAD'],
|
||||
['git', 'checkout', 'main'],
|
||||
['git', 'pull'],
|
||||
[pip_path, 'install', '--upgrade', 'pip', 'setuptools', 'wheel'],
|
||||
[pip_path, 'install', '--editable', '.'],
|
||||
],
|
||||
[call[0] for call in subprocess_calls],
|
||||
|
||||
@@ -15,6 +15,9 @@ from ffx.process import ( # noqa: E402
|
||||
COMMAND_NOT_FOUND_RETURN_CODE,
|
||||
COMMAND_TIMED_OUT_RETURN_CODE,
|
||||
executeProcess,
|
||||
getWrappedCommandSequence,
|
||||
normalizeCpuPercent,
|
||||
normalizeNiceness,
|
||||
)
|
||||
|
||||
|
||||
@@ -47,6 +50,39 @@ class ProcessTests(unittest.TestCase):
|
||||
self.assertIn("Command timed out", err)
|
||||
self.assertIn(sys.executable, err)
|
||||
|
||||
def test_get_wrapped_command_sequence_leaves_command_unwrapped_when_limits_disabled(self):
|
||||
wrapped = getWrappedCommandSequence(
|
||||
["ffmpeg", "-i", "input.mkv"],
|
||||
context={"resource_limits": {"niceness": None, "cpu_percent": None}},
|
||||
)
|
||||
|
||||
self.assertEqual(["ffmpeg", "-i", "input.mkv"], wrapped)
|
||||
|
||||
def test_get_wrapped_command_sequence_wraps_nice_when_configured(self):
|
||||
wrapped = getWrappedCommandSequence(
|
||||
["ffmpeg", "-i", "input.mkv"],
|
||||
context={"resource_limits": {"niceness": 5, "cpu_percent": None}},
|
||||
)
|
||||
|
||||
self.assertEqual(["nice", "-n", "5", "ffmpeg", "-i", "input.mkv"], wrapped)
|
||||
|
||||
def test_get_wrapped_command_sequence_wraps_cpulimit_around_nice_when_both_configured(self):
|
||||
wrapped = getWrappedCommandSequence(
|
||||
["ffmpeg", "-i", "input.mkv"],
|
||||
context={"resource_limits": {"niceness": 5, "cpu_percent": 42}},
|
||||
)
|
||||
|
||||
self.assertEqual(
|
||||
["cpulimit", "-l", "42", "--", "nice", "-n", "5", "ffmpeg", "-i", "input.mkv"],
|
||||
wrapped,
|
||||
)
|
||||
|
||||
def test_normalize_niceness_accepts_disabled_sentinel(self):
|
||||
self.assertIsNone(normalizeNiceness(99))
|
||||
|
||||
def test_normalize_cpu_percent_accepts_disabled_sentinel(self):
|
||||
self.assertIsNone(normalizeCpuPercent(0))
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
Reference in New Issue
Block a user