-
Notifications
You must be signed in to change notification settings - Fork 5.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Needs thorough testing] async model file listing #4968
Open
mcmonkey4eva
wants to merge
20
commits into
comfyanonymous:master
Choose a base branch
from
mcmonkey4eva:async-file-list
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+71
−26
Open
Changes from 4 commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
34430b3
[proof of concept] async model file listing
mcmonkey4eva d6ba7ed
oops, fix sloppy refactor
mcmonkey4eva e98b52b
other fix
mcmonkey4eva 2deb55d
one more cleanup
mcmonkey4eva b35ad9a
discard aiofiles
mcmonkey4eva b8022cf
use a threading.Event to make intent more explicit
mcmonkey4eva 5d8a0b7
simplify async code with ThreadPoolExecutor
mcmonkey4eva 163bfff
`.shutdown` isn't so friendly :(
mcmonkey4eva a256213
Merge branch 'master' into async-file-list
mcmonkey4eva d659be7
minor format
mcmonkey4eva 49f973b
minor improvement
mcmonkey4eva c3d47aa
clearer error message
mcmonkey4eva 5a00fea
handle python lambda behaviors
mcmonkey4eva 3c3ef31
Merge branch 'master' into async-file-list
mcmonkey4eva 00db33f
use one persistent thread pool executor instead of generating new ones
mcmonkey4eva 587d0c9
add a max_workers value
mcmonkey4eva ba90c60
pre-scan model lists
mcmonkey4eva 409f4b6
minor cleanup
mcmonkey4eva 6fca6e1
prebuild should have its own executor
mcmonkey4eva 67a69da
pretty that up a tad
mcmonkey4eva File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only the first element of
must_invalidate
is accessed. Is there any reason why it needs to be an array?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
python lets you read any variable anywhere very freely, but writing has a lot of edge cases and oddities, which hit you when you're doing threading stuff especially. But if you have an array, then it's just a read you're doing in the thread, and the value inside the array is somewhere in the heap down yonder, so it lets you do it. So the array is being used as the equivalent to a pointer/reference essentially
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems really hacky. Please try use
threading.Event
to approach this problem.It replaces the must_invalidate list hack with a threading.Event object, which is designed for inter-thread communication.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fair enough, swapped to Event