Haidra-Org / horde-worker-reGen

The default client software to create images for the AI-Horde
https://aihorde.net/
GNU Affero General Public License v3.0
93 stars 42 forks source link

feat: latest comfyui; fix: better GPU utilization for SD15 #270

Closed tazlin closed 1 month ago

tazlin commented 2 months ago

Changes/fixes:

tazlin commented 1 month ago

@CodiumAI-Agent /review

CodiumAI-Agent commented 1 month ago

PR Reviewer Guide πŸ”

(Review updated until commit https://github.com/Haidra-Org/horde-worker-reGen/commit/f89812c17a691e606061ca5eb2d603b164b5df8a)

⏱️ Estimated effort to review: 4 πŸ”΅πŸ”΅πŸ”΅πŸ”΅βšͺ
πŸ§ͺ No relevant tests
πŸ”’ No security concerns identified
⚑ Key issues to review

Performance Concern
The method `remove_maintenance` is added to remove maintenance mode from a worker. However, the method makes synchronous network calls (`simple_client.worker_details_by_name` and `simple_client.worker_modify`) within an asynchronous context. This could block the event loop and affect the performance of the application. Consider refactoring these calls to be asynchronous or running them in a separate thread. Redundant Code
The method `_receive_and_handle_control_message` contains a condition to check if `message.control_flag` is `HordeControlFlag.START_INFERENCE` and then preloads a model if not already active. However, the method `preload_model` is called again inside the condition, which seems redundant and could lead to unnecessary preloading of the model. This could be optimized to avoid potential performance issues. Configuration Overlap
The `start_inference_process` function has parameters `low_memory_mode`, `high_memory_mode`, and `very_high_memory_mode` that could potentially overlap in functionality. This might lead to confusing behavior depending on how these flags are set. It's recommended to clarify the precedence and interaction of these modes in the documentation or refactor the approach to handle memory management settings more clearly.
CodiumAI-Agent commented 1 month ago

Persistent review updated to latest commit https://github.com/Haidra-Org/horde-worker-reGen/commit/f89812c17a691e606061ca5eb2d603b164b5df8a