From bb0d27bc146b74e4048ad9a1600a0aa9bc4d32ba Mon Sep 17 00:00:00 2001 From: blissful Date: Mon, 13 May 2024 16:35:47 -0400 Subject: [PATCH] improve cli startup time performance by making more work lazy --- rose-cli/rose_cli/cli.py | 7 +++++-- rose-py/rose/audiotags.py | 28 ++++++++++++++++++++-------- rose-py/rose/config.py | 19 ++++++++++++------- rose-py/rose/config_test.py | 9 --------- rose-py/rose/templates.py | 35 +++++++++++++++++++++++++---------- rose-zig/build.zig | 11 ++++++----- 6 files changed, 68 insertions(+), 41 deletions(-) diff --git a/rose-cli/rose_cli/cli.py b/rose-cli/rose_cli/cli.py index 229a4a1..8696ee8 100644 --- a/rose-cli/rose_cli/cli.py +++ b/rose-cli/rose_cli/cli.py @@ -50,8 +50,6 @@ toggle_release_new, update_cache, ) -from rose_vfs import mount_virtualfs -from rose_watch import start_watchdog from rose_cli.dump import ( dump_all_artists, @@ -169,6 +167,7 @@ def update(ctx: Context, force: bool) -> None: @click.pass_obj def watch(ctx: Context, foreground: bool) -> None: """Start a watchdog to auto-update the cache when the source directory changes.""" + from rose_watch import start_watchdog if not foreground: daemonize(pid_path=ctx.config.watchdog_pid_path) @@ -203,6 +202,10 @@ def fs() -> None: @click.pass_obj def mount(ctx: Context, foreground: bool) -> None: """Mount the virtual filesystem.""" + from rose_vfs import mount_virtualfs + + ctx.config.validate_path_templates_expensive() + if not foreground: daemonize() diff --git a/rose-py/rose/audiotags.py b/rose-py/rose/audiotags.py index 339d6dd..dedb21d 100644 --- a/rose-py/rose/audiotags.py +++ b/rose-py/rose/audiotags.py @@ -17,14 +17,6 @@ from pathlib import Path from typing import Any, no_type_check -import mutagen -import mutagen.flac -import mutagen.id3 -import mutagen.mp3 -import mutagen.mp4 -import mutagen.oggopus -import mutagen.oggvorbis - from rose.common import Artist, ArtistMapping, RoseError, RoseExpectedError, uniq if typing.TYPE_CHECKING: @@ -135,6 +127,14 @@ class AudioTags: @classmethod def from_file(cls, p: Path) -> AudioTags: """Read the tags of an audio file on disk.""" + import mutagen + import mutagen.flac + import mutagen.id3 + import mutagen.mp3 + import mutagen.mp4 + import mutagen.oggopus + import mutagen.oggvorbis + if not any(p.suffix.lower() == ext for ext in SUPPORTED_AUDIO_EXTENSIONS): raise UnsupportedFiletypeError(f"{p.suffix} not a supported filetype") try: @@ -310,6 +310,14 @@ def _get_paired_frame(x: str) -> str | None: @no_type_check def flush(self, *, validate: bool = True) -> None: """Flush the current tags to the file on disk.""" + import mutagen + import mutagen.flac + import mutagen.id3 + import mutagen.mp3 + import mutagen.mp4 + import mutagen.oggopus + import mutagen.oggvorbis + m = mutagen.File(self.path) if not validate and "pytest" not in sys.modules: raise Exception("Validate can only be turned off by tests.") @@ -477,6 +485,8 @@ def _split_tag(t: str | None) -> list[str]: def _get_tag(t: Any, keys: list[str], *, split: bool = False, first: bool = False) -> str | None: + import mutagen.id3 + if not t: return None for k in keys: @@ -503,6 +513,8 @@ def _get_tag(t: Any, keys: list[str], *, split: bool = False, first: bool = Fals def _get_tuple_tag(t: Any, keys: list[str]) -> tuple[str, str] | tuple[None, None]: + import mutagen.id3 + if not t: return None, None for k in keys: diff --git a/rose-py/rose/config.py b/rose-py/rose/config.py index 794c530..d85c1b9 100644 --- a/rose-py/rose/config.py +++ b/rose-py/rose/config.py @@ -552,13 +552,6 @@ def parse(cls, config_path_override: Path | None = None) -> Config: if not data["path_templates"]: del data["path_templates"] - try: - path_templates.parse() - except InvalidPathTemplateError as e: - raise InvalidConfigValueError( - f"Invalid path template in configuration file ({cfgpath}) for template {e.key}: {e}" - ) from e - vfs_config = VirtualFSConfig.parse(cfgpath, data.get("vfs", {})) if data: @@ -606,3 +599,15 @@ def cache_database_path(self) -> Path: @functools.cached_property def watchdog_pid_path(self) -> Path: return self.cache_dir / "watchdog.pid" + + def validate_path_templates_expensive(self) -> None: + """ + Validate all the path templates. This is expensive, so we don't do it when reading the + configuration, only on demand. + """ + try: + self.path_templates.parse() + except InvalidPathTemplateError as e: + raise InvalidConfigValueError( + f"Invalid path template in for template {e.key}: {e}" + ) from e diff --git a/rose-py/rose/config_test.py b/rose-py/rose/config_test.py index ff872d8..3ebd45a 100644 --- a/rose-py/rose/config_test.py +++ b/rose-py/rose/config_test.py @@ -468,15 +468,6 @@ def write(x: str) -> None: """ ) - # path_templates - write(config + '\npath_templates.source.release = "{% if hi %}{{"') - with pytest.raises(InvalidConfigValueError) as excinfo: - Config.parse(config_path_override=path) - assert ( - str(excinfo.value) - == f"Invalid path template in configuration file ({path}) for template source.release: Failed to compile template: unexpected 'end of template'" - ) - # rename_source_files write(config + '\nrename_source_files = "lalala"') with pytest.raises(InvalidConfigValueError) as excinfo: diff --git a/rose-py/rose/templates.py b/rose-py/rose/templates.py index 39e29c0..f226413 100644 --- a/rose-py/rose/templates.py +++ b/rose-py/rose/templates.py @@ -14,12 +14,12 @@ from functools import cached_property from typing import Any -import jinja2 - from rose.audiotags import RoseDate from rose.common import Artist, ArtistMapping, RoseExpectedError if typing.TYPE_CHECKING: + import jinja2 + from rose.cache import Release, Track from rose.config import Config @@ -96,13 +96,26 @@ def lastname(x: str) -> str: return x -ENVIRONMENT = jinja2.Environment() -ENVIRONMENT.filters["arrayfmt"] = arrayfmt -ENVIRONMENT.filters["artistsarrayfmt"] = artistsarrayfmt -ENVIRONMENT.filters["artistsfmt"] = artistsfmt -ENVIRONMENT.filters["releasetypefmt"] = releasetypefmt -ENVIRONMENT.filters["sortorder"] = sortorder -ENVIRONMENT.filters["lastname"] = lastname +# Global variable cache for a lazy initialization. We lazily initialize the Jinja environment to +# improve the CLI startup time. +__environment: jinja2.Environment | None = None + + +def get_environment() -> jinja2.Environment: + global __environment + if __environment: + return __environment + + import jinja2 + + __environment = jinja2.Environment() + __environment.filters["arrayfmt"] = arrayfmt + __environment.filters["artistsarrayfmt"] = artistsarrayfmt + __environment.filters["artistsfmt"] = artistsfmt + __environment.filters["releasetypefmt"] = releasetypefmt + __environment.filters["sortorder"] = sortorder + __environment.filters["lastname"] = lastname + return __environment class InvalidPathTemplateError(RoseExpectedError): @@ -122,7 +135,7 @@ class PathTemplate: @cached_property def compiled(self) -> jinja2.Template: - return ENVIRONMENT.from_string(self.text) + return get_environment().from_string(self.text) def __hash__(self) -> int: return hash(self.text) @@ -236,6 +249,8 @@ def parse(self) -> None: Attempt to parse all the templates into Jinja templates (which will be cached on the cached properties). This will raise an InvalidPathTemplateError if a template is invalid. """ + import jinja2 + key = "" try: key = "source.release" diff --git a/rose-zig/build.zig b/rose-zig/build.zig index 8306375..d8f487a 100644 --- a/rose-zig/build.zig +++ b/rose-zig/build.zig @@ -9,10 +9,11 @@ pub fn build(b: *std.Build) void { .target = target, .optimize = optimize, }); - const ffmpeg = b.dependency("ffmpeg", .{ - .target = target, - .optimize = optimize, - }); + // TODO: This is really expensive, so uncomment it only when we start using it. + // const ffmpeg = b.dependency("ffmpeg", .{ + // .target = target, + // .optimize = optimize, + // }); // Specify the core library module. const rose = b.addModule("rose", .{ @@ -20,7 +21,7 @@ pub fn build(b: *std.Build) void { .target = target, .optimize = optimize, .imports = &[_]std.Build.Module.Import{ - .{ .name = "av", .module = ffmpeg.module("av") }, + // .{ .name = "av", .module = ffmpeg.module("av") }, .{ .name = "sqlite", .module = sqlite.module("sqlite") }, }, });