Skip to content

feat: add libuv as optional runtime#228

Closed
NSPC911 wants to merge 3 commits intomasterfrom
libuv
Closed

feat: add libuv as optional runtime#228
NSPC911 wants to merge 3 commits intomasterfrom
libuv

Conversation

@NSPC911
Copy link
Owner

@NSPC911 NSPC911 commented Feb 27, 2026

Summary by CodeRabbit

  • New Features

    • Added optional platform-specific event loop libraries for installation and an attempt to enable them at startup (falls back silently if unavailable).
  • Chores

    • Improved platform detection and initialization to explicitly support Windows, macOS, Linux, and Android.

@coderabbitai

This comment has been minimized.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/rovr/__main__.py (1)

393-396: Consider adding a log message when libuv is successfully enabled.

For debugging and user awareness, it may be helpful to log when the alternative event loop is activated, especially since this is an optional feature. However, this is entirely optional.

💡 Optional: Add debug logging
     try:
         import asyncio

         if sys.platform == "win32":
             import winloop as libuv
         else:
             import uvloop as libuv
         asyncio.set_event_loop_policy(libuv.EventLoopPolicy())
+        if is_dev:
+            pprint("  [dim]Using libuv event loop[/]")
     except (ModuleNotFoundError, ImportError):
         pass
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/rovr/__main__.py` around lines 393 - 396, Add a debug/info log when an
alternative event loop is enabled: after the sys.platform check that imports
winloop or uvloop into the name libuv, call the module logger to emit a message
like "enabled winloop" or "enabled uvloop" (use the same logger used elsewhere
in the module, e.g., getLogger(__name__) or the existing module logger) so it's
clear which event loop was activated; place the log immediately after the import
in the if/else branches referencing sys.platform, winloop, uvloop and libuv.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/rovr/__main__.py`:
- Around line 393-396: Add a debug/info log when an alternative event loop is
enabled: after the sys.platform check that imports winloop or uvloop into the
name libuv, call the module logger to emit a message like "enabled winloop" or
"enabled uvloop" (use the same logger used elsewhere in the module, e.g.,
getLogger(__name__) or the existing module logger) so it's clear which event
loop was activated; place the log immediately after the import in the if/else
branches referencing sys.platform, winloop, uvloop and libuv.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2e63433 and a64329f.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (3)
  • pyproject.toml
  • src/rovr/__main__.py
  • src/rovr/variables/constants.py

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds an optional “libuv” runtime extra to use a faster event loop implementation (uvloop on Unix-like platforms, winloop on Windows) and wires it into the CLI entrypoint so it’s enabled when installed.

Changes:

  • Added libuv optional dependency extra (uvloop/winloop) to packaging metadata and lockfile.
  • Added best-effort runtime initialization of the libuv-backed asyncio event loop policy at startup.
  • Updated platform detection for os_type to use sys.platform matching.

Reviewed changes

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

File Description
uv.lock Locks new optional extra dependencies uvloop / winloop under libuv.
src/rovr/variables/constants.py Switches os_type derivation from platform.system() to sys.platform match + explicit mapping.
src/rovr/main.py Attempts to import uvloop/winloop and set the asyncio event loop policy when available.
pyproject.toml Defines the libuv extra for uvloop/winloop with platform markers.

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

Comment on lines +147 to +157
match sys.platform:
case "win32":
os_type = "Windows"
case "darwin":
os_type = "Darwin"
case linux if linux.startswith("linux"):
os_type = "Linux"
case "android":
os_type = "Linux"
case _:
raise NotImplementedError(f"Unsupported platform: {sys.platform}")
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

constants.py is imported widely (e.g., rovr.variables.constants.os_type). Changing the default branch to raise NotImplementedError introduces a hard import-time failure on any unrecognized sys.platform (e.g., *bsd/cygwin), whereas previously platform.system() would return a string and allow the app to start. Consider falling back to platform.system() (or a safe default like "Linux"/sys.platform) instead of raising at import time, unless this is an intentional breaking change documented elsewhere.

Copilot uses AI. Check for mistakes.
else:
import uvloop as libuv # ty: ignore[unresolved-import]
asyncio.set_event_loop_policy(libuv.EventLoopPolicy())
except (ModuleNotFoundError, ImportError):
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

ModuleNotFoundError is a subclass of ImportError, so except (ModuleNotFoundError, ImportError) is redundant here. Catching only ImportError is equivalent and simpler.

Suggested change
except (ModuleNotFoundError, ImportError):
except ImportError:

Copilot uses AI. Check for mistakes.
@NSPC911
Copy link
Owner Author

NSPC911 commented Feb 27, 2026

@coderabbitai do you think this is worth it?

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 27, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

# Check what kind of application rovr is
cat README.md 2>/dev/null || echo "No README.md found"

# Look for async/await usage in the codebase to understand if it's async-heavy
echo "=== Async usage patterns ==="
rg -n "async def|await " --type=py -C1 | head -50

# Check the main entry point to understand the application better
echo "=== Main entry point ==="
cat src/rovr/__main__.py

# Look for asyncio usage patterns
echo "=== Asyncio usage ==="
rg -n "asyncio\." --type=py | head -30

# Check pyproject.toml for project metadata
echo "=== Project metadata ==="
cat pyproject.toml

Length of output: 34826


🌐 Web query:

Textual framework uvloop compatibility Python TUI

💡 Result:

Yes—Textual is asyncio-based, so it can run on uvloop anywhere uvloop is supported (Unix-like OSes). Textual’s App.run() accepts an explicit loop (AbstractEventLoop) you can supply. [1][2]

Practical pattern (recommended vs. global policy)

Create a uvloop loop and pass it to Textual:

import asyncio
import uvloop
from my_app import MyApp

if __name__ == "__main__":
    loop = uvloop.new_event_loop()
    asyncio.set_event_loop(loop)
    MyApp().run(loop=loop)

This avoids relying on asyncio.set_event_loop_policy(...), which is deprecated in Python 3.14 and planned for removal in Python 3.16. [3]

Platform notes

  • uvloop does not support Windows (so on Windows, use default asyncio, or consider an alternative like winloop if you specifically need a faster loop). [4][5]

Sources: [1] Textual App.run(loop=...) API, [2] Textual uses asyncio, [3] Python docs on deprecating event loop policies, [4] uvloop Windows limitation, [5] winloop project.


@NSPC911 Here's my honest assessment:

⚖️ Trade-offs Analysis

Potential Benefits

  • Performance gains for I/O operations (uvloop is typically 2-4x faster for servers)
  • Your async usage includes subprocess spawning (zoxide, rg, fd searches) which could benefit
  • Clean optional dependency with graceful fallback ✅

Concerns

1. Questionable impact for TUI workloads
Textual is asyncio-based and can run on uvloop, but the performance gains are most pronounced in high-throughput server scenarios. For a TUI app where async operations are:

  • User-triggered searches (occasional, not continuous)
  • Subprocess spawning (bottleneck is the external tool, not the event loop)
  • UI updates (already optimized by Textual)

The real-world performance improvement may be imperceptible to users.

2. Deprecated implementation pattern ⚠️
Your current approach uses asyncio.set_event_loop_policy(), but this is deprecated in Python 3.14 and planned for removal in Python 3.16. Since you support Python 3.12-3.13, this will break soon.

Better approach (if you proceed):

# In __main__.py, replace the policy setup with:
try:
    if sys.platform == "win32":
        import winloop as libuv
    else:
        import uvloop as libuv
    loop = libuv.new_event_loop()
    asyncio.set_event_loop(loop)
    # Then pass loop to Application if it accepts it
except ImportError:
    loop = None

Recommendation

Without benchmarks showing measurable improvement, I'd say this adds complexity for uncertain gains. Consider:

  1. Run benchmarks first: Compare search operations, app startup, and responsiveness with/without uvloop
  2. If improvements are <10% for typical operations → probably not worth it
  3. If you see significant gains → fix the deprecation issue before merging

What specific performance issues are you trying to address? That would help determine if this is the right solution.

@NSPC911
Copy link
Owner Author

NSPC911 commented Feb 27, 2026

Will be closing it for now, hopefully I don't delete this branch...

@NSPC911 NSPC911 closed this Feb 27, 2026
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