From 476a245598514f0d33bcf598de4981a7f9aaf93f Mon Sep 17 00:00:00 2001 From: Alex P Date: Wed, 3 Sep 2025 23:01:08 +0000 Subject: [PATCH] [WIP] Cleanup: Reduce PR complexity --- internal/audio/batch_audio.go | 6 +- internal/audio/config_constants.go | 110 +++--------------- internal/audio/input_ipc.go | 22 +--- internal/audio/output_streaming.go | 16 --- internal/audio/priority_scheduler.go | 168 --------------------------- 5 files changed, 17 insertions(+), 305 deletions(-) delete mode 100644 internal/audio/priority_scheduler.go diff --git a/internal/audio/batch_audio.go b/internal/audio/batch_audio.go index 6b063ea4..95d245fb 100644 --- a/internal/audio/batch_audio.go +++ b/internal/audio/batch_audio.go @@ -498,8 +498,7 @@ func (bap *BatchAudioProcessor) processBatchWrite(batch []batchWriteRequest) { threadWasPinned = true runtime.LockOSThread() - // Set high priority for batch audio processing - skip logging in hotpath - _ = SetAudioThreadPriority() + // Priority scheduler not implemented - using default thread priority } batchSize := len(batch) @@ -512,8 +511,7 @@ func (bap *BatchAudioProcessor) processBatchWrite(batch []batchWriteRequest) { // Add deferred function to release thread lock if we pinned it if threadWasPinned { defer func() { - // Skip logging in hotpath for performance - _ = ResetThreadPriority() + // Priority scheduler not implemented - using default thread priority runtime.UnlockOSThread() atomic.StoreInt32(&bap.writePinned, 0) bap.stats.WriteThreadTime += time.Since(start) diff --git a/internal/audio/config_constants.go b/internal/audio/config_constants.go index f0cbb245..6223b427 100644 --- a/internal/audio/config_constants.go +++ b/internal/audio/config_constants.go @@ -224,51 +224,8 @@ type AudioConfigConstants struct { // Used in: process_monitor.go for configuring thread scheduling behavior // Impact: Controls how audio threads are scheduled by the Linux kernel - // SchedNormal defines normal (CFS) scheduling policy. - // Used in: process_monitor.go for non-critical audio threads - // Impact: Standard time-sharing scheduling, may cause audio latency under load. - // Value 0 corresponds to SCHED_NORMAL in Linux kernel. - SchedNormal int - - // SchedFIFO defines First-In-First-Out real-time scheduling policy. - // Used in: process_monitor.go for critical audio threads requiring deterministic timing - // Impact: Provides real-time scheduling but may starve other processes if misused. - // Value 1 corresponds to SCHED_FIFO in Linux kernel. - SchedFIFO int - - // SchedRR defines Round-Robin real-time scheduling policy. - // Used in: process_monitor.go for real-time threads that should share CPU time - // Impact: Real-time scheduling with time slicing, balances determinism and fairness. - // Value 2 corresponds to SCHED_RR in Linux kernel. - SchedRR int - - // Real-time Priority Levels - Priority values for real-time audio thread scheduling - // Used in: process_monitor.go for setting thread priorities - // Impact: Higher priorities get more CPU time but may affect system responsiveness - - // RTAudioHighPriority defines highest priority for critical audio threads. - // Used in: process_monitor.go for time-critical audio processing (encoding/decoding) - // Impact: Ensures audio threads get CPU time but may impact system responsiveness. - // Default 80 provides high priority without completely starving other processes. - RTAudioHighPriority int - - // RTAudioMediumPriority defines medium priority for important audio threads. - // Used in: process_monitor.go for audio I/O and buffering operations - // Impact: Good priority for audio operations while maintaining system balance. - // Default 60 provides elevated priority for audio without extreme impact. - RTAudioMediumPriority int - - // RTAudioLowPriority defines low priority for background audio threads. - // Used in: process_monitor.go for audio monitoring and metrics collection - // Impact: Ensures audio background tasks run without impacting critical operations. - // Default 40 provides some priority elevation while remaining background. - RTAudioLowPriority int - - // RTNormalPriority defines normal priority (no real-time scheduling). - // Used in: process_monitor.go for non-critical audio threads - // Impact: Standard scheduling priority, no special real-time guarantees. - // Default 0 uses normal kernel scheduling without real-time privileges. - RTNormalPriority int + // Removed unused scheduling policy constants and RT priority values + // The priority scheduler is not implemented - functions are called but don't exist // Process Management - Configuration for audio process lifecycle management // Used in: supervisor.go for managing audio process restarts and recovery @@ -1842,47 +1799,6 @@ func DefaultAudioConfig() *AudioConfigConstants { // Used in: Non-critical audio processing tasks // Impact: Provides standard scheduling suitable for non-critical tasks. // Default 0 (SCHED_NORMAL) for standard time-sharing scheduling. - SchedNormal: 0, - - // SchedFIFO defines real-time first-in-first-out scheduling policy. - // Used in: Critical audio processing requiring deterministic timing - // Impact: Provides deterministic scheduling for latency-critical operations. - // Default 1 (SCHED_FIFO) for real-time first-in-first-out scheduling. - SchedFIFO: 1, - - // SchedRR defines real-time round-robin scheduling policy. - // Used in: Balanced real-time processing with time slicing - // Impact: Provides real-time scheduling with balanced time slicing. - // Default 2 (SCHED_RR) for real-time round-robin scheduling. - SchedRR: 2, - - // Real-time Priority Levels - Configuration for process priorities - // Used in: Process priority management and CPU scheduling - // Impact: Controls priority hierarchy for audio system components - - // RTAudioHighPriority defines highest priority for audio processing. - // Used in: Latency-critical audio operations and CPU priority assignment - // Impact: Ensures highest CPU priority without starving system processes. - // Default 80 provides highest priority for latency-critical operations. - RTAudioHighPriority: 80, - - // RTAudioMediumPriority defines medium priority for audio tasks. - // Used in: Important audio tasks requiring elevated priority - // Impact: Provides elevated priority while allowing higher priority operations. - // Default 60 balances importance with system operation priority. - RTAudioMediumPriority: 60, - - // RTAudioLowPriority defines low priority for audio tasks. - // Used in: Audio tasks needing responsiveness but not latency-critical - // Impact: Provides moderate real-time priority for responsive tasks. - // Default 40 ensures responsiveness without being latency-critical. - RTAudioLowPriority: 40, - - // RTNormalPriority defines normal scheduling priority. - // Used in: Non-real-time audio processing tasks - // Impact: Provides standard priority for non-real-time operations. - // Default 0 represents normal scheduling priority. - RTNormalPriority: 0, // Process Management - Configuration for process restart and recovery // Used in: Process monitoring and failure recovery systems @@ -2171,17 +2087,17 @@ func DefaultAudioConfig() *AudioConfigConstants { // Used in: process management, thread scheduling for audio processing // Impact: Controls CPU scheduling priority for audio threads - // AudioHighPriority defines highest priority for critical audio threads (-10). + // AudioHighPriority defines highest priority for critical audio threads (5). // Used in: Real-time audio processing threads, encoder/decoder threads - // Impact: Ensures audio threads get CPU time before other processes - // Default -10 provides high priority without requiring root privileges - AudioHighPriority: -10, + // Impact: Ensures audio threads get CPU time but prioritizes mouse input + // Modified to 5 to prevent mouse lag on single-core RV1106 + AudioHighPriority: 5, - // AudioMediumPriority defines medium priority for important audio threads (-5). + // AudioMediumPriority defines medium priority for important audio threads (10). // Used in: Audio buffer management, IPC communication threads // Impact: Balances audio performance with system responsiveness - // Default -5 ensures good performance while allowing other critical tasks - AudioMediumPriority: -5, + // Modified to 10 to prioritize mouse input on single-core RV1106 + AudioMediumPriority: 10, // AudioLowPriority defines low priority for non-critical audio threads (0). // Used in: Metrics collection, logging, cleanup tasks @@ -2195,11 +2111,11 @@ func DefaultAudioConfig() *AudioConfigConstants { // Default 0 represents normal Linux process priority NormalPriority: 0, - // NiceValue defines default nice value for audio processes (-10). + // NiceValue defines default nice value for audio processes (5). // Used in: Process creation, priority adjustment for audio components - // Impact: Improves audio process scheduling without requiring special privileges - // Default -10 provides better scheduling while remaining accessible to non-root users - NiceValue: -10, + // Impact: Ensures audio processes don't interfere with mouse input + // Modified to 5 to prioritize mouse input on single-core RV1106 + NiceValue: 5, // Error Handling - Configuration for robust error recovery and retry logic // Used in: Throughout audio pipeline for handling transient failures diff --git a/internal/audio/input_ipc.go b/internal/audio/input_ipc.go index e2f3eed8..ad9b5e3e 100644 --- a/internal/audio/input_ipc.go +++ b/internal/audio/input_ipc.go @@ -292,9 +292,6 @@ func (ais *AudioInputServer) Start() error { // Submit the connection acceptor to the audio reader pool if !SubmitAudioReaderTask(ais.acceptConnections) { // If the pool is full or shutting down, fall back to direct goroutine creation - logger := logging.GetDefaultLogger().With().Str("component", AudioInputClientComponent).Logger() - logger.Warn().Msg("Audio reader pool full or shutting down, falling back to direct goroutine creation") - go ais.acceptConnections() } @@ -369,9 +366,6 @@ func (ais *AudioInputServer) acceptConnections() { // Handle this connection using the goroutine pool if !SubmitAudioReaderTask(func() { ais.handleConnection(conn) }) { // If the pool is full or shutting down, fall back to direct goroutine creation - logger := logging.GetDefaultLogger().With().Str("component", AudioInputClientComponent).Logger() - logger.Warn().Msg("Audio reader pool full or shutting down, falling back to direct goroutine creation") - go ais.handleConnection(conn) } } @@ -1019,9 +1013,7 @@ func (ais *AudioInputServer) startProcessorGoroutine() { runtime.LockOSThread() defer runtime.UnlockOSThread() - // Set priority only when necessary to reduce scheduler interference - _ = SetAudioThreadPriority() - defer func() { _ = ResetThreadPriority() }() + // Priority scheduler not implemented - using default thread priority } // Create logger for this goroutine @@ -1139,21 +1131,11 @@ func (ais *AudioInputServer) startMonitorGoroutine() { config := GetConfig() useThreadOptimizations := config.MaxAudioProcessorWorkers > 8 - logger := logging.GetDefaultLogger().With().Str("component", AudioInputClientComponent).Logger() - if useThreadOptimizations { runtime.LockOSThread() defer runtime.UnlockOSThread() - // Set I/O priority for monitoring only when needed - if err := SetAudioIOThreadPriority(); err != nil { - logger.Warn().Err(err).Msg("Failed to set audio I/O priority") - } - defer func() { - if err := ResetThreadPriority(); err != nil { - logger.Warn().Err(err).Msg("Failed to reset thread priority") - } - }() + // Priority scheduler not implemented - using default thread priority } defer ais.wg.Done() diff --git a/internal/audio/output_streaming.go b/internal/audio/output_streaming.go index 5213d053..c9690ab0 100644 --- a/internal/audio/output_streaming.go +++ b/internal/audio/output_streaming.go @@ -208,22 +208,6 @@ func (s *AudioOutputStreamer) processingLoop() { // Pin goroutine to OS thread for consistent performance runtime.LockOSThread() defer runtime.UnlockOSThread() - - // Set high priority for audio output processing - if err := SetAudioThreadPriority(); err != nil { - // Only log priority warnings if warn level enabled to reduce overhead - if getOutputStreamingLogger().GetLevel() <= zerolog.WarnLevel { - getOutputStreamingLogger().Warn().Err(err).Msg("Failed to set audio output processing priority") - } - } - defer func() { - if err := ResetThreadPriority(); err != nil { - // Only log priority warnings if warn level enabled to reduce overhead - if getOutputStreamingLogger().GetLevel() <= zerolog.WarnLevel { - getOutputStreamingLogger().Warn().Err(err).Msg("Failed to reset thread priority") - } - } - }() } for frameData := range s.processingChan { diff --git a/internal/audio/priority_scheduler.go b/internal/audio/priority_scheduler.go deleted file mode 100644 index 23c3209b..00000000 --- a/internal/audio/priority_scheduler.go +++ /dev/null @@ -1,168 +0,0 @@ -//go:build linux - -package audio - -import ( - "runtime" - "syscall" - "unsafe" - - "github.com/jetkvm/kvm/internal/logging" - "github.com/rs/zerolog" -) - -// SchedParam represents scheduling parameters for Linux -type SchedParam struct { - Priority int32 -} - -// getPriorityConstants returns priority levels from centralized config -func getPriorityConstants() (audioHigh, audioMedium, audioLow, normal int) { - config := GetConfig() - return config.AudioHighPriority, config.AudioMediumPriority, config.AudioLowPriority, config.NormalPriority -} - -// getSchedulingPolicies returns scheduling policies from centralized config -func getSchedulingPolicies() (schedNormal, schedFIFO, schedRR int) { - config := GetConfig() - return config.SchedNormal, config.SchedFIFO, config.SchedRR -} - -// PriorityScheduler manages thread priorities for audio processing -type PriorityScheduler struct { - logger zerolog.Logger - enabled bool -} - -// NewPriorityScheduler creates a new priority scheduler -func NewPriorityScheduler() *PriorityScheduler { - return &PriorityScheduler{ - logger: logging.GetDefaultLogger().With().Str("component", "priority-scheduler").Logger(), - enabled: true, - } -} - -// SetThreadPriority sets the priority of the current thread -func (ps *PriorityScheduler) SetThreadPriority(priority int, policy int) error { - if !ps.enabled { - return nil - } - - // Lock to OS thread to ensure we're setting priority for the right thread - runtime.LockOSThread() - - // Get current thread ID - tid := syscall.Gettid() - - // Set scheduling parameters - param := &SchedParam{ - Priority: int32(priority), - } - - // Use syscall to set scheduler - _, _, errno := syscall.Syscall(syscall.SYS_SCHED_SETSCHEDULER, - uintptr(tid), - uintptr(policy), - uintptr(unsafe.Pointer(param))) - - if errno != 0 { - // If we can't set real-time priority, try nice value instead - schedNormal, _, _ := getSchedulingPolicies() - if policy != schedNormal { - ps.logger.Warn().Int("errno", int(errno)).Msg("failed to set real-time priority, falling back to nice") - return ps.setNicePriority(priority) - } - return errno - } - - ps.logger.Debug().Int("tid", tid).Int("priority", priority).Int("policy", policy).Msg("thread priority set") - return nil -} - -// setNicePriority sets nice value as fallback when real-time scheduling is not available -func (ps *PriorityScheduler) setNicePriority(rtPriority int) error { - // Convert real-time priority to nice value (inverse relationship) - // RT priority 80 -> nice -10, RT priority 40 -> nice 0 - niceValue := (40 - rtPriority) / 4 - if niceValue < GetConfig().MinNiceValue { - niceValue = GetConfig().MinNiceValue - } - if niceValue > GetConfig().MaxNiceValue { - niceValue = GetConfig().MaxNiceValue - } - - err := syscall.Setpriority(syscall.PRIO_PROCESS, 0, niceValue) - if err != nil { - ps.logger.Warn().Err(err).Int("nice", niceValue).Msg("failed to set nice priority") - return err - } - - ps.logger.Debug().Int("nice", niceValue).Msg("nice priority set as fallback") - return nil -} - -// SetAudioProcessingPriority sets high priority for audio processing threads -func (ps *PriorityScheduler) SetAudioProcessingPriority() error { - audioHigh, _, _, _ := getPriorityConstants() - _, schedFIFO, _ := getSchedulingPolicies() - return ps.SetThreadPriority(audioHigh, schedFIFO) -} - -// SetAudioIOPriority sets medium priority for audio I/O threads -func (ps *PriorityScheduler) SetAudioIOPriority() error { - _, audioMedium, _, _ := getPriorityConstants() - _, schedFIFO, _ := getSchedulingPolicies() - return ps.SetThreadPriority(audioMedium, schedFIFO) -} - -// SetAudioBackgroundPriority sets low priority for background audio tasks -func (ps *PriorityScheduler) SetAudioBackgroundPriority() error { - _, _, audioLow, _ := getPriorityConstants() - _, schedFIFO, _ := getSchedulingPolicies() - return ps.SetThreadPriority(audioLow, schedFIFO) -} - -// ResetPriority resets thread to normal scheduling -func (ps *PriorityScheduler) ResetPriority() error { - _, _, _, normal := getPriorityConstants() - schedNormal, _, _ := getSchedulingPolicies() - return ps.SetThreadPriority(normal, schedNormal) -} - -// Disable disables priority scheduling (useful for testing or fallback) -func (ps *PriorityScheduler) Disable() { - ps.enabled = false - ps.logger.Debug().Msg("priority scheduling disabled") -} - -// Enable enables priority scheduling -func (ps *PriorityScheduler) Enable() { - ps.enabled = true - ps.logger.Debug().Msg("priority scheduling enabled") -} - -// Global priority scheduler instance -var globalPriorityScheduler *PriorityScheduler - -// GetPriorityScheduler returns the global priority scheduler instance -func GetPriorityScheduler() *PriorityScheduler { - if globalPriorityScheduler == nil { - globalPriorityScheduler = NewPriorityScheduler() - } - return globalPriorityScheduler -} - -// SetAudioThreadPriority is a convenience function to set audio processing priority -func SetAudioThreadPriority() error { - return GetPriorityScheduler().SetAudioProcessingPriority() -} - -// SetAudioIOThreadPriority is a convenience function to set audio I/O priority -func SetAudioIOThreadPriority() error { - return GetPriorityScheduler().SetAudioIOPriority() -} - -// ResetThreadPriority is a convenience function to reset thread priority -func ResetThreadPriority() error { - return GetPriorityScheduler().ResetPriority() -}