diff --git a/ovos_workshop/resource_files.py b/ovos_workshop/resource_files.py index 8b3ddabe..d67f4e26 100644 --- a/ovos_workshop/resource_files.py +++ b/ovos_workshop/resource_files.py @@ -17,6 +17,8 @@ import json import re from collections import namedtuple +import os +from functools import lru_cache from os import walk from os.path import dirname from pathlib import Path @@ -93,19 +95,117 @@ def match_lang_directories(lang: str, base_dirs: List[Path]) -> List[Path]: """ candidates = [] for directory in base_dirs: - if directory.exists(): - for folder in directory.iterdir(): - if folder.is_dir(): - # ovos_spec_tools owns the language-distance policy and - # its threshold; a directory whose name is not a usable - # match for `lang` scores beyond it and is skipped. - if lang_matches(lang, folder.name): - candidates.append((folder, lang_distance(lang, folder.name))) + for name in _subdirectory_names(directory): + # ovos_spec_tools owns the language-distance policy and its + # threshold; a directory whose name is not a usable match for + # `lang` scores beyond it and is skipped. + distance = _lang_dir_distance(lang, name) + if distance is not None: + candidates.append((Path(directory, name), distance)) # sort by distance to target lang code candidates = sorted(candidates, key=lambda k: k[1]) return [c[0] for c in candidates] +# A skill resolves its resources once per (language, resource type) and once +# per resource file, and a skill that ships dozens of locales paid for listing +# and language-matching every one of them on each call: on a hub serving 11 +# languages that was ~65 directories x ~26 lookups x 11 languages per skill, +# plus an os.walk per intent/vocab/dialog file. Package directories do not +# change while they are installed, and a reinstall replaces them (new inode, +# new mtime), so both are memoized on (path, inode, mtime). +_DIR_CACHE_MAX = 4096 +_subdir_cache: Dict[str, Tuple[Tuple[int, int], Tuple[str, ...]]] = {} +_file_index_cache: Dict[str, Tuple[tuple, Dict[str, Path]]] = {} + + +def _dir_identity(directory) -> Optional[Tuple[int, int]]: + try: + st = os.stat(directory) + except OSError: + return None + return st.st_ino, st.st_mtime_ns + + +def _subdirectory_names(directory) -> Tuple[str, ...]: + """Names of the immediate subdirectories of `directory`, memoized.""" + key = str(directory) + identity = _dir_identity(key) + if identity is None: + return () + cached = _subdir_cache.get(key) + if cached is not None and cached[0] == identity: + return cached[1] + try: + with os.scandir(key) as entries: + names = tuple(sorted(e.name for e in entries if e.is_dir())) + except OSError: + return () + if len(_subdir_cache) >= _DIR_CACHE_MAX: + _subdir_cache.clear() + _subdir_cache[key] = (identity, names) + return names + + +@lru_cache(maxsize=8192) +def _lang_dir_distance(lang: str, name: str) -> Optional[float]: + """lang_distance for a usable language directory name, else None.""" + if not lang_matches(lang, name): + return None + return lang_distance(lang, name) + + +def _tree_still_matches(signature) -> bool: + """True when every directory and file recorded in `signature` still has + the identity it had when the cached result was built. + + A directory's mtime moves when an entry is added, removed or renamed in + it -- but only in it, not in its subdirectories -- so a recursive cache + records every directory it walked, and every file whose content it used. + """ + for kind, path, identity in signature: + try: + st = os.stat(path) + except OSError: + return False + if kind == "d": + current = (st.st_ino, st.st_mtime_ns) + else: + current = (st.st_ino, st.st_mtime_ns, st.st_size) + if current != identity: + return False + return True + + +def _walk_file_index(directory) -> Dict[str, Path]: + """file name -> path for every file under `directory`, memoized. + + Reproduces what ResourceFile._locate's walk returned: when several + subdirectories hold the same file name, the one the walk reaches last. + The cached index is reused only while every directory the walk visited + is unchanged, so a file added anywhere in the tree is seen. + """ + key = str(directory) + cached = _file_index_cache.get(key) + if cached is not None and _tree_still_matches(cached[0]): + return cached[1] + index: Dict[str, Path] = {} + signature = [] + for root, _, file_names in walk(key): + identity = _dir_identity(root) + if identity is None: + continue + signature.append(("d", root, identity)) + for file_name in file_names: + index[file_name] = Path(root, file_name) + if not signature: + return {} + if len(_file_index_cache) >= _DIR_CACHE_MAX: + _file_index_cache.clear() + _file_index_cache[key] = (tuple(signature), index) + return index + + def _foreign_lang_dirs(directory: Path, root_dir: str, lang: Optional[str]) -> set: """ @@ -347,19 +447,16 @@ def _locate(self) -> Optional[str]: if file_name in file_names: file_path = Path(directory, file_name) - # check the skill resources - if file_path is None: - walk_directory = str(self.resource_type.base_directory) - for directory, _, file_names in walk(walk_directory): - if file_name in file_names: - file_path = Path(directory, file_name) + # check the skill resources (package data: indexed once per + # directory instead of walked once per resource file) + if file_path is None and self.resource_type.base_directory is not None: + file_path = _walk_file_index( + self.resource_type.base_directory).get(file_name) # check ovos-workshop resources if file_path is None and self.resource_type.workshop_directory is not None: - walk_directory = str(self.resource_type.workshop_directory) - for directory, _, file_names in walk(walk_directory): - if file_name in file_names: - file_path = Path(directory, file_name) + file_path = _walk_file_index( + self.resource_type.workshop_directory).get(file_name) if file_path is None: LOG.debug(f"Could not find resource file {file_name} for lang: {self.resource_type.language}") @@ -811,14 +908,44 @@ def _locale_vocabularies(self) -> Dict[str, List[str]]: vocabulary of that name. User overrides take precedence over the skill's own resources, which take precedence over workshop-shipped resources. """ - vocabularies: Dict[str, List[str]] = {} + # Called for every .intent and .blacklist file the skill loads, and + # each call walked the vocabulary tree and read and expanded every + # .voc in it again. The map is reused while every directory walked + # and every .voc read still has the identity it had (one stat each, + # no reads); a user override directory is never cached. vocab_type = self.types.vocabulary + cacheable = not vocab_type.user_directory + if cacheable: + memo = getattr(self, "_vocabularies_memo", None) + if memo is not None and memo[0] == (vocab_type.base_directory, + vocab_type.workshop_directory) \ + and _tree_still_matches(memo[1]): + return {name: list(members) for name, members in memo[2].items()} + signature: list = [] + vocabularies = self._read_locale_vocabularies(vocab_type, signature) + if cacheable: + self._vocabularies_memo = ( + (vocab_type.base_directory, vocab_type.workshop_directory), + tuple(signature), + {name: list(members) for name, members in vocabularies.items()}) + return vocabularies + + def _read_locale_vocabularies(self, vocab_type, + signature: Optional[list] = None + ) -> Dict[str, List[str]]: + """Read every .voc in the vocabulary tree; when `signature` is given, + append the identity of each directory walked and each file read.""" + vocabularies: Dict[str, List[str]] = {} for base in (vocab_type.user_directory, vocab_type.base_directory, vocab_type.workshop_directory): if not base: continue for directory, _, files in walk(str(base)): + if signature is not None: + identity = _dir_identity(directory) + if identity is not None: + signature.append(("d", directory, identity)) for file_name in files: if not file_name.endswith(".voc"): continue @@ -826,7 +953,15 @@ def _locale_vocabularies(self) -> Dict[str, List[str]]: if name in vocabularies: continue members: List[str] = [] - with open(Path(directory, file_name)) as voc_file: + voc_path = Path(directory, file_name) + if signature is not None: + try: + st = os.stat(voc_path) + signature.append(("f", str(voc_path), + (st.st_ino, st.st_mtime_ns, st.st_size))) + except OSError: + pass + with open(voc_path) as voc_file: for line in (ln.strip().lower() for ln in voc_file): if line and not line.startswith("#"): # a .voc line MAY hold comma/parenthesis diff --git a/test/unittests/test_resource_lookup_cache.py b/test/unittests/test_resource_lookup_cache.py new file mode 100644 index 00000000..c025daad --- /dev/null +++ b/test/unittests/test_resource_lookup_cache.py @@ -0,0 +1,147 @@ +"""Resource lookup memoizes package directory listings without going stale.""" +import os +import tempfile +import time +import unittest +from pathlib import Path +from unittest import mock + +from ovos_workshop import resource_files +from ovos_workshop.resource_files import (ResourceType, match_lang_directories, + locate_lang_directories) + + +def _touch(path: Path, text: str = "x\n"): + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text) + + +class TestResourceLookupCache(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.skill = Path(self.tmp.name) + for lang in ("en-US", "fr-FR", "de-DE", "es-ES"): + _touch(self.skill / "locale" / lang / "intents" / "hello.intent") + resource_files._subdir_cache.clear() + resource_files._file_index_cache.clear() + + def tearDown(self): + self.tmp.cleanup() + + def test_same_result_as_uncached(self): + self.assertEqual(locate_lang_directories("fr-FR", str(self.skill)), + [self.skill / "locale" / "fr-FR"]) + self.assertEqual(locate_lang_directories("en-US", str(self.skill)), + [self.skill / "locale" / "en-US"]) + + def test_listing_is_reused(self): + locate_lang_directories("fr-FR", str(self.skill)) + with mock.patch.object(resource_files.os, "scandir", + side_effect=AssertionError("relisted")): + for lang in ("fr-FR", "de-DE", "en-US"): + locate_lang_directories(lang, str(self.skill)) + + def test_new_language_directory_is_seen(self): + self.assertEqual(locate_lang_directories("it-IT", str(self.skill)), []) + time.sleep(0.01) + _touch(self.skill / "locale" / "it-IT" / "intents" / "hello.intent") + # the locale directory's mtime moved, so the listing is refreshed + self.assertEqual(locate_lang_directories("it-IT", str(self.skill)), + [self.skill / "locale" / "it-IT"]) + + def test_resource_file_located_once_per_directory(self): + rt = ResourceType("intent", ".intent", "fr-FR") + rt.locate_base_directory(str(self.skill)) + first = resource_files.IntentFile(rt, "hello").file_path + self.assertEqual(first, self.skill / "locale" / "fr-FR" / "intents" / "hello.intent") + with mock.patch.object(resource_files, "walk", + side_effect=AssertionError("walked again")): + again = resource_files.IntentFile(rt, "hello.intent").file_path + missing = resource_files.IntentFile(rt, "nope").file_path + self.assertEqual(again, first) + self.assertIsNone(missing) + + def test_missing_directory(self): + self.assertEqual(match_lang_directories("en-US", [self.skill / "absent"]), []) + + +if __name__ == "__main__": + unittest.main() + + +class TestLocaleVocabulariesMemo(unittest.TestCase): + def setUp(self): + from ovos_workshop.resource_files import SkillResources + self.tmp = tempfile.TemporaryDirectory() + self.skill = Path(self.tmp.name) + _touch(self.skill / "locale" / "en-US" / "vocab" / "colour.voc", "Red\nblue\n") + _touch(self.skill / "locale" / "en-US" / "intents" / "paint.intent", "paint it \n") + resource_files._subdir_cache.clear() + resource_files._file_index_cache.clear() + self.resources = SkillResources(str(self.skill), "en-US", skill_id="test.skill") + + def tearDown(self): + self.tmp.cleanup() + + def test_read_once_and_copied(self): + first = self.resources._locale_vocabularies() + self.assertEqual(first["colour"], ["red", "blue"]) # workshop's own .voc ride along + expected = {name: list(members) for name, members in first.items()} + first["colour"].append("mutated") + with mock.patch.object(resource_files, "walk", + side_effect=AssertionError("walked again")): + second = self.resources._locale_vocabularies() + self.assertEqual(second, expected) + + def test_user_override_directory_is_not_cached(self): + self.resources._locale_vocabularies() + self.resources.types.vocabulary.user_directory = self.skill / "locale" / "en-US" + with mock.patch.object(resource_files, "walk", wraps=resource_files.walk) as spy: + self.resources._locale_vocabularies() + self.assertTrue(spy.called) + + +class TestCachesSeeChangesDeepInTheTree(unittest.TestCase): + """A cache validated by the root's identity alone would miss these.""" + + def setUp(self): + from ovos_workshop.resource_files import SkillResources + self.tmp = tempfile.TemporaryDirectory() + self.skill = Path(self.tmp.name) + _touch(self.skill / "locale" / "en-US" / "vocab" / "colour.voc", "red\n") + _touch(self.skill / "locale" / "en-US" / "intents" / "paint.intent", "paint it \n") + resource_files._subdir_cache.clear() + resource_files._file_index_cache.clear() + self.resources = SkillResources(str(self.skill), "en-US", skill_id="test.skill") + + def tearDown(self): + self.tmp.cleanup() + + def test_file_added_in_a_subdirectory_is_found(self): + rt = ResourceType("intent", ".intent", "en-US") + rt.locate_base_directory(str(self.skill)) + self.assertIsNone(resource_files.IntentFile(rt, "erase").file_path) + root_before = os.stat(self.skill / "locale" / "en-US").st_mtime_ns + time.sleep(0.01) + _touch(self.skill / "locale" / "en-US" / "intents" / "erase.intent", "erase it\n") + # the language directory itself did not change... + self.assertEqual(os.stat(self.skill / "locale" / "en-US").st_mtime_ns, root_before) + # ...but the subdirectory did, and the index notices + self.assertEqual(resource_files.IntentFile(rt, "erase").file_path, + self.skill / "locale" / "en-US" / "intents" / "erase.intent") + + def test_vocabulary_added_in_a_subdirectory_is_seen(self): + self.assertNotIn("size", self.resources._locale_vocabularies()) + time.sleep(0.01) + _touch(self.skill / "locale" / "en-US" / "vocab" / "size.voc", "big\n") + self.assertEqual(self.resources._locale_vocabularies()["size"], ["big"]) + + def test_vocabulary_edited_in_place_is_reread(self): + self.assertEqual(self.resources._locale_vocabularies()["colour"], ["red"]) + voc = self.skill / "locale" / "en-US" / "vocab" / "colour.voc" + dir_before = os.stat(voc.parent).st_mtime_ns + time.sleep(0.01) + with open(voc, "w") as f: # same inode, same directory entry + f.write("red\ngreen\n") + self.assertEqual(os.stat(voc.parent).st_mtime_ns, dir_before) + self.assertEqual(self.resources._locale_vocabularies()["colour"], ["red", "green"])