Skip to content

Integrate dynamic batching and in-memory audio cache features - #7

Open
hdduytran wants to merge 6 commits into
maemreyo:mainfrom
hdduytran:integrate/batching-and-cache
Open

Integrate dynamic batching and in-memory audio cache features#7
hdduytran wants to merge 6 commits into
maemreyo:mainfrom
hdduytran:integrate/batching-and-cache

Conversation

@hdduytran

Copy link
Copy Markdown

merge: integrate dynamic batching + in-memory audio cache

What this does

Merges two independently developed feature branches into main:

  1. feature/dynamic-batching — groups concurrent requests into single batched model.generate() calls
  2. feature/in-memory-cache — LRU cache with TTL for repeated synthesis requests

Both branches were based off main independently (no stacking). This integration branch merges them sequentially, resolves conflicts, and verifies the combined result.

Why a merge branch

The two features touch overlapping files (config.py, app.py, health.py, README.md, docs/architecture/overview.md, docs/design/dataflow.md) but are functionally independent. Merging via an integration branch lets us resolve all 6 conflicts in one place and verify the combined behavior before landing on main.

Conflicts resolved

File Conflict Resolution
config.py Both added fields after request_timeout_s Combined: batching fields first, then cache fields
app.py Both added imports and shutdown logic Combined: import both services, shutdown both in sequence
routers/health.py Both added fields to /health response Combined: batch config + cache config in same response
README.md Both added feature bullet and Advanced Features section Combined: both bullets, both sections (batching then cache)
docs/architecture/overview.md Both modified service layer diagram, component map, dependency graph, startup/shutdown sequence Combined: BatchingService + AudioCache in all diagrams
docs/design/dataflow.md Both modified /health and /metrics JSON examples Combined: all fields from both features in responses

How the features interact

They don't — by design. The request flow is:

Request → Cache lookup → HIT? return cached bytes
                       → MISS? → BatchingService (or legacy single-request)
                                → model.generate()
                                → encode audio bytes
                                → Cache store
                                → return to client

Cache sits at the HTTP layer (keyed on request params, stores final bytes). Batching sits at the inference layer (groups model.generate() calls). They compose naturally with no coupling.

Merge history

main (ec8b4a4)
├── feature/dynamic-batching (2 commits)
│   ├── feat: add dynamic batching for inference
│   └── docs: add dynamic batching documentation
└── feature/in-memory-cache (2 commits)
    ├── feat: add in-memory LRU cache for repeated synthesis requests
    └── docs: add audio cache documentation

integrate/batching-and-cache
├── merge: integrate dynamic batching feature (no conflicts)
└── merge: integrate in-memory audio cache feature (6 conflicts resolved)

Verification

  • 0 conflict markers remaining (grep -rn "<<<<<<" — clean)
  • 0 diagnostics issues across all 7 modified source files
  • 40/40 tests passing

TraiPPN added 6 commits April 6, 2026 07:52
Leverage OmniVoice's native batch API (model.generate(text=[...]))
to group concurrent requests into single GPU calls, improving
throughput under load.

- Add BatchingService with configurable batch_max_size and
  batch_timeout_ms, background dispatch loop, and per-parameter-group
  batching with sequential fallback on upstream API errors
- Add batch_semaphore to cap concurrent batch dispatches to
  max_concurrent, preventing unbounded executor queue growth
- Wire BatchingService into InferenceService and app lifespan
  with graceful shutdown
- Expose --batch-enabled, --batch-max-size, --batch-timeout-ms
  CLI flags and OMNIVOICE_BATCH_* env vars
- Add batch metrics (batches_dispatched, avg_batch_size) to
  /metrics endpoint and batch config to /health
- Clarify max_concurrent semantics: controls thread pool size
  for both batched and single-request modes
- Disable batching in test fixtures (tests mock synthesize directly)
Cache encoded audio bytes (WAV/PCM) keyed on the full parameter set
(text, voice mode, profile path, speed, num_step, etc.) to serve
identical requests instantly without re-running inference.

- Add AudioCache with LRU eviction, configurable max memory cap
  (cache_max_mb, default 512MB), and TTL expiry (cache_ttl_s,
  default 1h)
- Skip caching entries larger than the entire budget to prevent
  thrashing
- Cache key uses SHA-256 of all output-affecting parameters;
  clone mode uses stable profile path (not temp file path)
- Integrate into /v1/audio/speech (non-streaming only); streaming
  and one-shot /clone (temp file paths) are not cached
- Add X-Cache: HIT/MISS response header for observability
- Expose cache stats (entries, bytes, hit rate, evictions) on
  /metrics and cache config on /health
- Add --cache-enabled, --cache-max-mb, --cache-ttl-s CLI flags
  and OMNIVOICE_CACHE_* env vars
Update README, architecture overview, and dataflow docs to cover:
- AudioCache in layer diagram, component map, and service graph
- Cache lookup/store in non-streaming request lifecycle
- DELETE /v1/audio/cache endpoint
- Cache stats in /health and /metrics responses
- Configuration table with cache CLI flags and env vars
- Audio Cache section under Advanced Features in README
Update README, architecture overview, and dataflow docs to cover:
- BatchingService in layer diagram, component map, and service graph
- Dual concurrency model diagrams (batched vs legacy)
- Updated key invariants table with batch_semaphore
- What happens under load for both modes
- Batch dispatch in startup/shutdown sequence
- Batch stats in /metrics and batch config in /health
- Configuration table with batch CLI flags and env vars
- Dynamic Batching section under Advanced Features in README
Resolve conflicts in config, app, health router, README, and docs
by combining both batching and cache features side-by-side.
@hdduytran
hdduytran requested a review from maemreyo as a code owner April 6, 2026 09:20

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The cache-before-batching ordering is exactly right! Short-circuiting early for cache hits avoids all the batching overhead. Have you considered adding metrics to track how often cache hits prevent batch formation? It could be interesting data for tuning the batch timeout!

Comment thread omnivoice_server/app.py

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The startup/shutdown sequence is well thought out. I like that batching service starts before cache - ensures inference pipeline is ready before caching layer. One minor thought: should the executor shutdown happen after cache cleanup? If cache cleanup triggers any async operations that need the executor, we might have issues. Though looking at the code, cache stop just cancels the sweep task so it's probably fine.

Comment thread README.md

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Having both features enabled by default is the right call for production deployments. One question: have you tested the interaction when both are enabled? Specifically:

  1. Cache hit rate impact on batch formation (fewer requests reach batching)
  2. Memory pressure with both caches (audio cache + batch queue)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

With both features enabled, a "cache prevented batches" metric would be useful - showing how many potential batch slots were saved by cache hits. Helps users understand the interaction between the two optimizations.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

With both features enabled, a "cache prevented batches" metric would be useful - showing how many potential batch slots were saved by cache hits. Helps users understand the interaction between the two optimizations.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The cache key coverage looks comprehensive. One edge case: if two requests have identical everything except response_format (WAV vs PCM), they'll be cached separately. This is correct behavior but worth noting in docs since the same inference produces different cached bytes

Comment thread omnivoice_server/cli.py

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

That's 6 new CLI flags - quite a lot! Have you considered grouping them?

# Current
omnivoice-server --batch-enabled --batch-max-size 16 --cache-max-mb 2048

Alternative: config profiles
omnivoice-server --performance-profile high-throughput

Not necessary for this PR, but food for thought for future UX improvements. The current explicit flags are clearer for power users.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The sequence diagrams showing cache lookup → batch accumulation → GPU dispatch are excellent! This makes the request lifecycle much clearer for new contributors. One suggestion: add a "cache hit" path diagram showing the short-circuit flow for completeness.

@maemreyo maemreyo left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Hi @hdduytran Im adding some feedback when walking through your excellent work. Please feel free to reach out, we can discuss more bout those.
Once again, thanks for your dedicated work on this!!

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