Command cache: fix update cache logic (#5539)

### Motivation

I noticed that when I type every letter in the prompt on the remote
server the typing is lagging. I thought it's ssh but no.
After inspection the commands cache code I found that on every key press
there is reading all 2000+ files in PATH because the code for cache
update has funny issue.

### Quiz

```xsh
def f():
    print(1)
    yield False
    print(2)
    yield True
    print(3)

print(any(f()))
```
What will be in the output? Answer:

<details>

```
1
2
True
```

The execution of `print(3)` ignored because `any()` interrupts the
execution of the function.
If we call `print(list(f()))` the output will be `1 2 3 [False, True]`.

</details>

### Before

* Updating cache (read all files in PATH and list of aliases) two times
after start.
* Updating cache (read all files in PATH and list of aliases) on every
key press in prompt.

### After

* Update cache once at start.
* Update cache only when we have real changes of paths or aliases.

cc #4954 #3895 #5309

## For community
⬇️ **Please click the 👍 reaction instead of leaving a `+1` or 👍
comment**

---------

Co-authored-by: a <1@1.1>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
This commit is contained in:
Andy Kipp 2024-06-26 08:32:51 +02:00 committed by GitHub
parent 4fb3c79ede
commit a52aa5febc
Failed to generate hash of commit
3 changed files with 46 additions and 9 deletions

View file

@ -0,0 +1,23 @@
**Added:**
* <news item>
**Changed:**
* <news item>
**Deprecated:**
* <news item>
**Removed:**
* <news item>
**Fixed:**
* Commands Cache: Fixed cache update logic that lead to lagging during typing.
**Security:**
* <news item>

View file

@ -187,9 +187,16 @@ def test_update_cache(xession, tmp_path):
file1.touch()
file1.chmod(0o755)
cache = CommandsCache({"PATH": [subdir2, subdir1]})
paths = [subdir2, subdir1]
cache = CommandsCache({"PATH": paths})
cached = cache.update_cache()
# Check there are no changes after update cache.
c1 = cache._update_and_check_changes(paths)
c2 = cache._update_and_check_changes(paths)
c3 = cache._update_and_check_changes(paths)
assert [c1, c2, c3] == [True, False, False]
assert file1.samefile(cached[basename][0])
# give the os enough time to update the mtime field of the parent directory

View file

@ -119,14 +119,21 @@ class CommandsCache(cabc.Mapping):
if os.path.isdir(p):
yield p
def _check_changes(self, paths: tuple[str, ...]):
# did PATH change?
yield self._update_paths_cache(paths)
def _update_aliases_cache(self):
"""Update aliases checksum and return result: updated or not."""
prev_hash = self._alias_checksum
self._alias_checksum = hash(frozenset(self.aliases))
return prev_hash != self._alias_checksum
# did aliases change?
al_hash = hash(frozenset(self.aliases))
yield al_hash != self._alias_checksum
self._alias_checksum = al_hash
def _update_and_check_changes(self, paths: tuple[str, ...]):
"""Update cache and return the result: updated or still the same.
Be careful in this place. Both `_update_*` functions must be called
because they are changing state after update.
"""
is_aliases_change = self._update_aliases_cache()
is_paths_change = self._update_paths_cache(paths)
return is_aliases_change or is_paths_change
@property
def all_commands(self):
@ -159,7 +166,7 @@ class CommandsCache(cabc.Mapping):
# iterate backwards so that entries at the front of PATH overwrite
# entries at the back.
paths = tuple(reversed(tuple(self.remove_dups(env.get("PATH") or []))))
if any(self._check_changes(paths)):
if self._update_and_check_changes(paths):
all_cmds = CacheDict()
for cmd, path in self._iter_binaries(paths):
# None -> not in aliases