bittensor icon indicating copy to clipboard operation
bittensor copied to clipboard

ThreadPoolExecutor -> Async Priority Queue (revolution)

Open ifrit98 opened this issue 2 years ago • 0 comments

Is your feature request related to a problem? Please describe.

While managing priorities for executing requests in our asynchronous context, we've found that using a threaded server might not be the most efficient approach. The combination of async programming and multi-threading can complicate the architecture and reduce the efficiency gains of asynchronous I/O.

For the revolution branch.

Describe the solution you'd like

Propose that we implement an asynchronous priority queue for managing task priorities. This would allow us to prioritize tasks and execute them based on their importance, all within the same event loop and without spawning additional threads.

Benefits:

  • Performance: By avoiding the overhead of thread management, we can get faster task processing.
  • Simplicity: This approach would be more in line with the asynchronous paradigm and could simplify our codebase.
  • Reliability: Reducing threading-related issues that might arise from the current approach.

Possible Implementation Approach: Use the PriorityQueue class from the asyncio library. It provides the necessary functionality for our requirements and integrates well with other asyncio components.

Describe alternatives you've considered

No response

Additional context

The issue with the current code revolves around the combined use of both async programming and multi-threading. These two approaches, while both useful for improving performance in different scenarios, can sometimes conflict when used together, complicating the architecture and potentially negating the benefits of async I/O.

Here are the points of concern from the provided code:

  1. Threaded Server:

    • File: axon.py
    class FastAPIThreadedServer(uvicorn.Server):
        ...
        @contextlib.contextmanager
        def run_in_thread(self):
            thread = threading.Thread(target=self.run, daemon=True)
            ...
    

    In the axon.py file, the FastAPIThreadedServer class runs the server in a separate thread. While this can help in some scenarios, when combined with async I/O (like making HTTP requests), it can introduce unnecessary complexity and potential performance issues.

  2. Thread Pool for Handling Requests:

    • File: axon.py
    self.thread_pool = bittensor.PriorityThreadPoolExecutor(
        max_workers=self.config.axon.max_workers
    )
    

    The axon class sets up a priority thread pool executor. While this provides a mechanism to prioritize tasks, it's a threaded approach. Given that many of the operations (especially network requests) in the provided codebase are asynchronous, it might be more efficient to handle these tasks asynchronously within the main event loop instead of delegating them to separate threads.

  3. Combined Use of Async and Threading:

    • File: dendrite.py
    try:
        loop = asyncio.get_event_loop()
        return loop.run_until_complete(self.forward(*args, **kwargs))
    except:
        new_loop = asyncio.new_event_loop()
        asyncio.set_event_loop(new_loop)
        result = loop.run_until_complete(self.forward(*args, **kwargs))
        new_loop.close()
        return result
    

    Here, in the dendrite.py file's query method, there's an attempt to run asynchronous tasks in the current event loop. If that fails, a new event loop is created using asyncio. This kind of switching between loops, especially within a threaded context, can lead to complications.

The main concern is the overlapping use of async functions and threading. While each has its own benefits, mixing them can introduce potential deadlocks, race conditions, or inefficiencies. By transitioning fully to an async approach with an asynchronous priority queue, many of these concerns could be alleviated, and the code might become simpler and more efficient.

ifrit98 avatar Aug 26 '23 03:08 ifrit98