Skip to content

Commit

Permalink
improve cli startup time performance by making more work lazy
Browse files Browse the repository at this point in the history
  • Loading branch information
azuline committed May 13, 2024
1 parent b5c356d commit d168498
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 42 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,4 @@ clean:
nixify-zig-deps:
cd rose-zig && zon2nix > deps.nix

.PHONY: help check build-zig test-py test-zig test typecheck lintcheck lint clean nixify-zig-deps
.PHONY: check build-zig test-py test-zig test typecheck lintcheck lint clean nixify-zig-deps
7 changes: 5 additions & 2 deletions rose-cli/rose_cli/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()

Expand Down
28 changes: 20 additions & 8 deletions rose-py/rose/audiotags.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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.")
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand Down
19 changes: 12 additions & 7 deletions rose-py/rose/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
9 changes: 0 additions & 9 deletions rose-py/rose/config_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
35 changes: 25 additions & 10 deletions rose-py/rose/templates.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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):
Expand All @@ -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)
Expand Down Expand Up @@ -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"
Expand Down
11 changes: 6 additions & 5 deletions rose-zig/build.zig
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,19 @@ 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", .{
.root_source_file = b.path("rose/root.zig"),
.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") },
},
});
Expand Down

0 comments on commit d168498

Please sign in to comment.