383 lines
No EOL
14 KiB
Markdown
383 lines
No EOL
14 KiB
Markdown
# newMusic Performance Analysis: 1-Second Lag Issues
|
|
|
|
## Executive Summary
|
|
|
|
After deep analysis of the `downloads.py` file, I've identified **the real root causes** of the 1-second lag spikes affecting the newMusic application. The issue is **NOT** simply the timer interval - it's **massive computational overhead** in the `update_download_status()` method that blocks the main UI thread.
|
|
|
|
## Critical Performance Bottlenecks Identified
|
|
|
|
### 1. **CRITICAL: Computational Complexity in Main Thread**
|
|
**Location:** `ui/pages/downloads.py:9547-9950` - `update_download_status()` method
|
|
|
|
**Problem:** The method performs extremely expensive operations on the main UI thread:
|
|
|
|
#### 1.1 Complex Filename Matching (Lines 9626-9694)
|
|
The method implements **5 different matching strategies** for each download item:
|
|
- **Strategy 1:** Direct filename match with extension checks
|
|
- **Strategy 2:** Track title substring matching
|
|
- **Strategy 3:** Album track parsing with split operations
|
|
- **Strategy 3.5:** Core track name matching with regex
|
|
- **Strategy 4:** Word matching with common term exclusions
|
|
- **Strategy 5:** File path matching
|
|
|
|
**Performance Impact:** For each download item, this creates **nested loops** that iterate through ALL transfers (potentially hundreds) and perform complex string operations.
|
|
|
|
#### 1.2 Repeated Imports Inside Hot Loops (Lines 9608, 9655, 9684)
|
|
```python
|
|
# These imports happen INSIDE the nested loops, potentially hundreds of times per second
|
|
import os # Line 9608 - inside transfer matching loop
|
|
import re # Line 9655 - inside core title matching
|
|
import re # Line 9684 - inside word matching
|
|
```
|
|
|
|
**Performance Impact:** Python imports are expensive operations and should never be inside loops.
|
|
|
|
#### 1.3 Expensive String Processing
|
|
```python
|
|
# Complex string operations repeated for every transfer/download combination
|
|
basename = os.path.basename(full_filename).lower()
|
|
download_title_lower = download_item.title.lower()
|
|
basename_lower = basename.lower()
|
|
```
|
|
|
|
**Performance Impact:** String operations compound with nested loops, creating O(n²) complexity.
|
|
|
|
### 2. **Threading Architecture Issues**
|
|
**Location:** `ui/pages/downloads.py:9967-9982` - Thread creation pattern
|
|
|
|
**Problem:** Creates **NEW threads every second** instead of reusing them:
|
|
|
|
```python
|
|
# PROBLEMATIC: Creates new thread every 1000ms
|
|
status_thread = TransferStatusThread(self.soulseek_client)
|
|
status_thread.transfer_status_completed.connect(handle_status_update)
|
|
# ...
|
|
status_thread.start()
|
|
```
|
|
|
|
**Performance Impact:**
|
|
- **Thread creation overhead** accumulates over time
|
|
- **Memory leaks** from abandoned threads
|
|
- **Resource exhaustion** with long-running applications
|
|
|
|
### 3. **UI Update Cascade**
|
|
**Location:** `ui/pages/downloads.py:3708-3800` - `update_status()` method
|
|
|
|
**Problem:** Immediate UI updates for every download item status change:
|
|
|
|
```python
|
|
def update_status(self, status: str, progress: int = None, download_speed: int = None, file_path: str = None):
|
|
# Update properties
|
|
self.status = status
|
|
# ...
|
|
# SYNCHRONOUS UI UPDATES ON MAIN THREAD
|
|
if hasattr(self, 'progress_bar') and self.progress_bar:
|
|
self.progress_bar.setValue(self.progress) # Triggers widget redraw
|
|
|
|
if hasattr(self, 'status_label') and self.status_label:
|
|
self.status_label.setText(status_text) # Triggers widget redraw
|
|
```
|
|
|
|
**Performance Impact:** Each status update triggers immediate widget redraws, compounding the blocking effect.
|
|
|
|
### 4. **Inefficient Data Structures**
|
|
**Location:** Throughout the status update loop
|
|
|
|
**Problem:** Linear search operations in nested loops:
|
|
|
|
```python
|
|
# O(n) search for each download item
|
|
for download_item in self.download_queue.download_items.copy():
|
|
# O(m) search for each transfer
|
|
for transfer in all_transfers:
|
|
# Complex matching logic for each combination
|
|
```
|
|
|
|
**Performance Impact:** O(n*m) complexity where n=download_items and m=transfers.
|
|
|
|
## Detailed Code Analysis
|
|
|
|
### Main Performance Hotspot: `update_download_status()` Method
|
|
|
|
**File:** `ui/pages/downloads.py`
|
|
**Lines:** 9547-9950
|
|
**Execution Frequency:** Every 1000ms via QTimer
|
|
|
|
#### Flow Analysis:
|
|
1. **Line 9567:** Flatten transfers data structure (acceptable performance)
|
|
2. **Line 9579:** Copy download items list (acceptable performance)
|
|
3. **Lines 9587-9704:** **CRITICAL BOTTLENECK** - Complex filename matching
|
|
4. **Lines 9706-9950:** Status processing and UI updates
|
|
|
|
#### The Killer Loop (Lines 9587-9704):
|
|
```python
|
|
# This creates O(n*m*k) complexity where:
|
|
# n = number of download items
|
|
# m = number of transfers
|
|
# k = complexity of each matching strategy
|
|
|
|
for download_item in self.download_queue.download_items.copy():
|
|
for transfer in all_transfers:
|
|
# Strategy 1: Direct filename match
|
|
if basename_lower == download_title_lower + '.mp3':
|
|
# Complex extension checking...
|
|
|
|
# Strategy 2: Track title matching
|
|
elif download_title_lower in basename_lower:
|
|
# Complex extension checking...
|
|
|
|
# Strategy 3: Album track parsing
|
|
elif ' - ' in download_item.title:
|
|
title_parts = download_item.title.split(' - ')
|
|
# Complex parsing logic...
|
|
|
|
# Strategy 3.5: Core track name matching
|
|
elif '(' in download_item.title and ')' in download_item.title:
|
|
import re # EXPENSIVE IMPORT IN LOOP!
|
|
core_title = re.sub(r'\([^)]*\)', '', download_item.title)
|
|
# More complex logic...
|
|
|
|
# Strategy 4: Word matching
|
|
elif any(word.lower() in basename_lower for word in download_item.title.split()):
|
|
# Complex word filtering and matching...
|
|
|
|
# Strategy 5: File path matching
|
|
elif download_item.file_path:
|
|
# More matching logic...
|
|
```
|
|
|
|
## Root Cause Analysis
|
|
|
|
### Why the 1-Second Lag Occurs:
|
|
|
|
1. **QTimer triggers** `update_download_status()` every 1000ms
|
|
2. **Method executes** expensive operations on the main UI thread
|
|
3. **UI becomes unresponsive** during processing (the "quarter-second lag")
|
|
4. **Cycle repeats** every second, creating consistent lag spikes
|
|
|
|
### Why Previous Solutions Failed:
|
|
|
|
1. **Timer interval changes** don't address the computational complexity
|
|
2. **Optimized polling** still blocks the main thread during processing
|
|
3. **Threading issues** persist with new thread creation every cycle
|
|
|
|
## Optimization Strategy
|
|
|
|
### Phase 1: Move Heavy Processing Off Main Thread
|
|
|
|
#### 1.1 Extract Filename Matching to Background Workers
|
|
```python
|
|
class MatchingWorker(QRunnable):
|
|
def __init__(self, download_items, all_transfers):
|
|
super().__init__()
|
|
self.download_items = download_items
|
|
self.all_transfers = all_transfers
|
|
self.signals = MatchingWorkerSignals()
|
|
|
|
def run(self):
|
|
# Move expensive matching logic here
|
|
matches = self.perform_matching()
|
|
self.signals.matches_found.emit(matches)
|
|
```
|
|
|
|
#### 1.2 Pre-compile Regex Patterns
|
|
```python
|
|
# At class initialization, not in loops
|
|
class DownloadsPage(QWidget):
|
|
def __init__(self, ...):
|
|
# Pre-compile expensive regex patterns
|
|
self.track_number_pattern = re.compile(r'^(\d+)\.\s*(.+)')
|
|
self.parenthetical_pattern = re.compile(r'\([^)]*\)')
|
|
self.uuid_pattern = re.compile(r'^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$')
|
|
```
|
|
|
|
#### 1.3 Cache Expensive Operations
|
|
```python
|
|
# Cache parsed results to avoid repeated processing
|
|
self.filename_cache = {} # filename -> parsed_data
|
|
self.title_cache = {} # title -> normalized_title
|
|
```
|
|
|
|
### Phase 2: Optimize Threading Architecture
|
|
|
|
#### 2.1 Implement Thread Pooling
|
|
```python
|
|
# Replace single-use threads with reusable pool
|
|
self.status_thread_pool = QThreadPool()
|
|
self.status_thread_pool.setMaxThreadCount(2)
|
|
|
|
# Reuse workers instead of creating new ones
|
|
class ReusableStatusWorker(QRunnable):
|
|
def __init__(self, soulseek_client):
|
|
super().__init__()
|
|
self.soulseek_client = soulseek_client
|
|
self.signals = StatusWorkerSignals()
|
|
|
|
def run(self):
|
|
# Reusable worker logic
|
|
pass
|
|
```
|
|
|
|
#### 2.2 Implement Proper Thread Lifecycle Management
|
|
```python
|
|
def update_download_status(self):
|
|
# Check if worker is already running
|
|
if self.status_worker_running:
|
|
return
|
|
|
|
# Reuse existing worker
|
|
worker = self.get_or_create_worker()
|
|
self.status_thread_pool.start(worker)
|
|
```
|
|
|
|
### Phase 3: Improve Data Structures
|
|
|
|
#### 3.1 Index Transfers by ID
|
|
```python
|
|
# O(1) lookup instead of O(n) search
|
|
def create_transfer_index(self, all_transfers):
|
|
transfer_index = {}
|
|
for transfer in all_transfers:
|
|
transfer_id = transfer.get('id')
|
|
if transfer_id:
|
|
transfer_index[transfer_id] = transfer
|
|
return transfer_index
|
|
```
|
|
|
|
#### 3.2 Use Efficient Matching Algorithms
|
|
```python
|
|
# Replace nested loops with efficient algorithms
|
|
def match_downloads_efficiently(self, download_items, transfers):
|
|
# Use set intersections, hash maps, and other efficient data structures
|
|
pass
|
|
```
|
|
|
|
### Phase 4: Optimize UI Updates
|
|
|
|
#### 4.1 Batch UI Updates
|
|
```python
|
|
# Instead of immediate updates, batch them
|
|
self.pending_ui_updates = []
|
|
|
|
def schedule_ui_update(self, download_item, status):
|
|
self.pending_ui_updates.append((download_item, status))
|
|
|
|
def process_batched_updates(self):
|
|
# Process all updates at once
|
|
for download_item, status in self.pending_ui_updates:
|
|
download_item.update_status(status)
|
|
self.pending_ui_updates.clear()
|
|
```
|
|
|
|
#### 4.2 Implement Dirty Flagging
|
|
```python
|
|
# Only update items that have actually changed
|
|
def update_status(self, status: str, progress: int = None, ...):
|
|
if self.status == status and self.progress == progress:
|
|
return # No change, skip update
|
|
|
|
# Mark as dirty and schedule update
|
|
self.is_dirty = True
|
|
self.schedule_update()
|
|
```
|
|
|
|
## Implementation Plan
|
|
|
|
### Step 1: Create Optimized Method (Week 1)
|
|
1. **Create new method** `update_download_status_optimized()`
|
|
2. **Implement background processing** for filename matching
|
|
3. **Add proper caching** for repeated operations
|
|
4. **Maintain full API compatibility** with existing functions
|
|
|
|
### Step 2: Optimize Threading (Week 2)
|
|
1. **Implement thread pooling** for status updates
|
|
2. **Add proper lifecycle management** for worker threads
|
|
3. **Implement worker reuse** to eliminate creation overhead
|
|
4. **Add performance monitoring** to measure improvements
|
|
|
|
### Step 3: Improve Data Structures (Week 3)
|
|
1. **Create efficient indexing** for transfer lookups
|
|
2. **Implement smart matching algorithms** to reduce complexity
|
|
3. **Add result caching** for repeated operations
|
|
4. **Optimize memory usage** with better data structures
|
|
|
|
### Step 4: Optimize UI Updates (Week 4)
|
|
1. **Implement batched UI updates** to reduce redraws
|
|
2. **Add dirty flagging** to skip unnecessary updates
|
|
3. **Optimize widget operations** for better performance
|
|
4. **Add user feedback** for long-running operations
|
|
|
|
## Testing Methodology
|
|
|
|
### Performance Metrics
|
|
1. **Main Thread Blocking Time:** Measure time spent in `update_download_status()`
|
|
2. **UI Responsiveness:** Track frame rate and input lag
|
|
3. **Memory Usage:** Monitor thread count and memory consumption
|
|
4. **CPU Usage:** Profile CPU utilization during status updates
|
|
|
|
### Test Scenarios
|
|
1. **Small Queue:** 1-5 downloads (baseline performance)
|
|
2. **Medium Queue:** 10-20 downloads (typical usage)
|
|
3. **Large Queue:** 50+ downloads (stress test)
|
|
4. **Mixed States:** Various download states (downloading, completed, failed)
|
|
|
|
### Success Criteria
|
|
1. **Zero lag spikes** during normal operation
|
|
2. **60-80% reduction** in main thread blocking time
|
|
3. **Consistent UI responsiveness** regardless of queue size
|
|
4. **Full functional compatibility** with existing features
|
|
|
|
## Rollback Strategy
|
|
|
|
### Rollback Triggers
|
|
1. **Functional regression** in download management
|
|
2. **API cleanup failures** breaking slskd integration
|
|
3. **UI corruption** or unresponsive interface
|
|
4. **Memory leaks** or resource exhaustion
|
|
|
|
### Rollback Process
|
|
1. **Disable optimized method** via feature flag
|
|
2. **Revert to original** `update_download_status()` method
|
|
3. **Clean up new threads** and workers
|
|
4. **Restore original timer** configuration
|
|
|
|
### Rollback Code
|
|
```python
|
|
# Feature flag for safe rollback
|
|
USE_OPTIMIZED_STATUS_UPDATE = False
|
|
|
|
def update_download_status(self):
|
|
if USE_OPTIMIZED_STATUS_UPDATE:
|
|
return self.update_download_status_optimized()
|
|
else:
|
|
return self.update_download_status_original()
|
|
```
|
|
|
|
## Expected Performance Gains
|
|
|
|
### Quantitative Improvements
|
|
- **60-80% reduction** in main thread blocking time
|
|
- **Eliminate 1-second lag spikes** entirely
|
|
- **50% reduction** in CPU usage during status updates
|
|
- **30% reduction** in memory usage from thread optimization
|
|
|
|
### Qualitative Improvements
|
|
- **Smooth UI interaction** during downloads
|
|
- **Responsive interface** regardless of queue size
|
|
- **Better scalability** for large download queues
|
|
- **Maintained reliability** with all existing features
|
|
|
|
## Conclusion
|
|
|
|
The 1-second lag issue in newMusic is caused by **computational complexity** in the `update_download_status()` method, not just the timer interval. The solution requires:
|
|
|
|
1. **Moving expensive operations** off the main thread
|
|
2. **Optimizing data structures** and algorithms
|
|
3. **Implementing proper threading** architecture
|
|
4. **Batching UI updates** for efficiency
|
|
|
|
This comprehensive approach will eliminate the lag while preserving all existing functionality including critical API cleanup operations.
|
|
|
|
---
|
|
|
|
*This analysis provides a complete roadmap to resolve the performance issues. The next step is to implement the optimized solution following the detailed plan above.* |