Skip to content

use a set instead of list for VAECache unprocessed file lookups#2841

Merged
bghira merged 1 commit into
mainfrom
chore/use-set-in-vaecache
Jul 17, 2026
Merged

use a set instead of list for VAECache unprocessed file lookups#2841
bghira merged 1 commit into
mainfrom
chore/use-set-in-vaecache

Conversation

@bghira

@bghira bghira commented Jul 17, 2026

Copy link
Copy Markdown
Owner

This pull request optimizes how the VAECache class handles and checks membership in the local_unprocessed_files list by introducing a cached set representation, which improves performance and ensures correctness when the file list changes. It also adds a unit test to verify this behavior.

Performance and correctness improvements:

  • Added a cached set (_local_unprocessed_files_set) and its source tracker to VAECache, with a helper method _local_unprocessed_file_set() that keeps the set in sync with local_unprocessed_files and is used for faster membership checks. [1] [2]
  • Updated methods (_reduce_bucket and process_buckets) to use the cached set for membership checks instead of the list, improving performance and correctness. [1] [2] [3]
  • Reset the cached set and its source to None whenever discover_unprocessed_files updates the file list, ensuring the cache stays accurate.

Testing:

  • Added a unit test test_local_unprocessed_file_set_tracks_reassigned_file_list to verify that the cached set properly tracks changes to local_unprocessed_files.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request improves VAECache membership-check performance by introducing a cached set view of local_unprocessed_files, keeping it in sync when the underlying list is reassigned, and updating the hot-path checks to use the set. It also adds a unit test to verify that the cache refreshes correctly when the list object changes.

Changes:

  • Added _local_unprocessed_files_set and _local_unprocessed_files_set_source plus _local_unprocessed_file_set() to cache a set derived from local_unprocessed_files.
  • Updated _reduce_bucket() and process_buckets() to use the cached set for membership checks instead of the list.
  • Added a unit test to ensure the cached set updates when local_unprocessed_files is reassigned.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
simpletuner/helpers/caching/vae.py Adds and uses a cached set for fast membership checks against local_unprocessed_files, with cache reset on rediscovery.
tests/test_vae.py Adds a unit test validating the cached set refreshes when the underlying list is replaced.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@bghira
bghira merged commit fb905c1 into main Jul 17, 2026
3 checks passed
@bghira
bghira deleted the chore/use-set-in-vaecache branch July 17, 2026 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants