[WIP][Serve] Fix perf regression in autoscaling snapshot cache refresh

[nadongjun]

Description

Improved controller performance degradation at scale (2048 replicas: loop_duration +15%, handle_metrics_delay +28%) introduced after PR #56225.

Root causes:

1. _refresh_autoscaling_deployments_cache() called list_deployment_details(), constructing O(N) ReplicaDetails Pydantic objects per deployment on every control loop
2. _create_deployment_snapshot() built a 14-field Pydantic DeploymentSnapshot on every autoscaling tick, regardless of whether scaling state had changed

Fixes:

1. Replaced list_deployment_details() with O(1) get_deployment() lookups
2. Replaced eager Pydantic construction with tuple key caching (~0.3us) + lazy DeploymentSnapshot construction (only when scaling state changes)
3. Separated _emit try/except from application state update to prevent error masking
4. Moved cache refresh after ASM timer to avoid polluting application_state_update_duration_s metrics
5. Removed dead code (_create_deployment_snapshot, is_scaling_equivalent)

Related issues

Re-lands #56225 (reverted in #61557) with performance fix.

Additional information

Benchmark

bench_fix_validation.py

Measured per-tick cost by importing actual Ray Serve modules (DeploymentAutoscalingState, get_decision_num_replicas(), etc.) without requiring a cluster. bench_fix_validation.py compares three code paths:

A) No snapshot (baseline) — autoscale logic only, no snapshot creation:

ctx = state.get_autoscaling_context(n)
decision, _ = state._policy(ctx)
decision = state.apply_bounds(decision)

B) Original PR #56225 — Pydantic DeploymentSnapshot created every tick:

ctx = state.get_autoscaling_context(n)
decision, _ = state._policy(ctx)
decision = state.apply_bounds(decision)
# 14-field Pydantic model constructed every tick (includes time.strftime, enum lookup)
DeploymentSnapshot(
    timestamp_str=time.strftime(...), app=..., deployment=...,
    current_replicas=..., target_replicas=..., min_replicas=...,
    max_replicas=..., scaling_status=..., policy_name=...,
    look_back_period_s=..., queued_requests=..., ongoing_requests=...,
    metrics_health=..., errors=...,
)

C) This fix — tuple cache + lazy Pydantic:

# Inside get_decision_num_replicas(): only caches tuples
state.get_decision_num_replicas(curr_target_num_replicas=n)
# In _emit(): tuple key comparison, Pydantic only built on state change
key = state.get_cached_snapshot_key()
if key != last_keys.get(dep_id):
    snap = state.get_deployment_snapshot()  # lazy Pydantic construction
    snap.dict(exclude_none=True)

create_state(n_replicas) creates a DeploymentAutoscalingState at 1–2048 replica scale. Each path is measured via time.perf_counter_ns() over 200 warmup + 2000 iterations:

  Replicas   A) No snapshot   B) Original #56225   C) This fix    B vs C
  --------   --------------   ------------------   -----------    ------
         1           3.2us               16.9us          7.0us     2.43x
        64           3.2us               16.5us          6.9us     2.38x
      1024           3.3us               17.0us          7.3us     2.34x
      2048           3.4us               17.1us          7.3us     2.35x

• Original PR overhead vs baseline: +405% (17.1us vs 3.4us)
• This fix overhead vs baseline: +115% (7.3us vs 3.4us, ~4us added)
• 2.35x faster than original PR, consistent across all replica counts
• O(1) per deployment: cost does not scale with replica count
• 4us added cost is < 0.01% of controller loop duration at 2048 replicas

Test results

All 11 tests in test_controller.py pass, including 4 snapshot-specific tests.

[gemini-code-assist]

Code Review

This pull request effectively addresses a performance regression in the autoscaling snapshot cache refresh by replacing expensive object constructions with efficient lookups. The introduction of dedicated logging for autoscaling snapshots is a great addition for observability. The code is well-structured, and the new tests are comprehensive.

I have a few minor suggestions to improve maintainability and clarity:

• Refactor duplicated code for updating timestamps in autoscaling_state.py.
• Simplify the is_scaling_equivalent method in common.py by removing redundant checks.
• Streamline the log parsing logic in the test file.

Overall, this is a solid improvement.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

[github-actions]

This pull request has been automatically marked as stale because it has not had
any activity for 14 days. It will be closed in another 14 days if no further activity occurs.
Thank you for your contributions.

You can always ask for help on our discussion forum or Ray's public slack channel.

If you'd like to keep this open, just leave any comment, and the stale label will be removed.

[nadongjun]

ping