diff --git a/MEMORY_OPTIMIZATION_RECOMMENDATIONS.md b/MEMORY_OPTIMIZATION_RECOMMENDATIONS.md new file mode 100644 index 0000000..a9a3b27 --- /dev/null +++ b/MEMORY_OPTIMIZATION_RECOMMENDATIONS.md @@ -0,0 +1,191 @@ +# Memory Optimization Recommendations for Allstarr + +## Current Implementation Status + +✅ **COMPLETED**: Mark-for-deletion system with 24-hour delay +✅ **COMPLETED**: Persistent favorites tracking using JSON files +✅ **COMPLETED**: Cache-first copying for favorites (avoids re-downloads) +✅ **COMPLETED**: Dependency injection for CacheCleanupService to process pending deletions + +## Memory Optimization Strategies + +### 1. Collection Optimizations + +**Current Issues:** +- Multiple `List`, `List`, `List` collections created during searches +- Large `Dictionary` objects for Jellyfin metadata +- Concurrent collections like `ConcurrentDictionary` for sessions + +**Recommendations:** +```csharp +// Use ArrayPool for temporary collections +private static readonly ArrayPool SongArrayPool = ArrayPool.Shared; + +// Use Span for temporary operations +ReadOnlySpan ProcessSongs(ReadOnlySpan songs) { ... } + +// Use IAsyncEnumerable for streaming large results +IAsyncEnumerable SearchSongsStreamAsync(string query); +``` + +### 2. JSON Serialization Optimizations + +**Current Issues:** +- Heavy use of `JsonSerializer.Deserialize>()` +- Multiple serialization/deserialization cycles for caching + +**Recommendations:** +```csharp +// Use System.Text.Json source generators for better performance +[JsonSerializable(typeof(List))] +[JsonSerializable(typeof(Dictionary))] +public partial class AllstarrJsonContext : JsonSerializerContext { } + +// Use JsonDocument for read-only scenarios instead of Dictionary +JsonDocument.Parse(json).RootElement.GetProperty("Items") +``` + +### 3. Caching Strategy Improvements + +**Current Issues:** +- File-based caching creates multiple copies of data +- Redis and file caches can contain duplicate data + +**Recommendations:** +```csharp +// Implement cache eviction policies +public class LRUCache where TKey : notnull +{ + private readonly int _maxSize; + private readonly Dictionary> _cache; + private readonly LinkedList _lruList; +} + +// Use weak references for large objects +private readonly WeakReference> _cachedSongs = new(null); +``` + +### 4. String Interning and Optimization + +**Current Issues:** +- Many duplicate strings (artist names, album titles) across collections +- Path strings created repeatedly + +**Recommendations:** +```csharp +// Use string interning for common values +private static readonly ConcurrentDictionary InternedStrings = new(); +public static string Intern(string value) => InternedStrings.GetOrAdd(value, v => v); + +// Use StringBuilder for path construction +private static readonly ThreadLocal PathBuilder = + new(() => new StringBuilder(256)); +``` + +### 5. Background Service Optimizations + +**Current Issues:** +- Multiple background services running simultaneously +- Potential memory leaks in long-running services + +**Recommendations:** +```csharp +// Implement proper disposal patterns +public class CacheCleanupService : BackgroundService, IDisposable +{ + private readonly SemaphoreSlim _semaphore = new(1, 1); + + protected override async Task ExecuteAsync(CancellationToken stoppingToken) + { + using var _ = await _semaphore.WaitAsync(stoppingToken); + // ... cleanup logic + } + + public override void Dispose() + { + _semaphore?.Dispose(); + base.Dispose(); + } +} +``` + +### 6. HTTP Client Optimizations + +**Current Issues:** +- Multiple HTTP clients for different services +- Large response buffers + +**Recommendations:** +```csharp +// Use HttpClientFactory with proper configuration +builder.Services.AddHttpClient(client => +{ + client.DefaultRequestHeaders.Add("User-Agent", "Allstarr/1.0"); + client.Timeout = TimeSpan.FromSeconds(30); +}) +.ConfigurePrimaryHttpMessageHandler(() => new HttpClientHandler +{ + MaxConnectionsPerServer = 10, + PooledConnectionLifetime = TimeSpan.FromMinutes(15) +}); + +// Stream large responses instead of loading into memory +using var stream = await response.Content.ReadAsStreamAsync(); +using var reader = new StreamReader(stream); +``` + +### 7. Container Memory Limits + +**Docker Configuration:** +```dockerfile +# Set memory limits in docker-compose.yml +services: + allstarr: + deploy: + resources: + limits: + memory: 512M + reservations: + memory: 256M +``` + +**Runtime Configuration:** +```csharp +// Configure GC for container environments +GCSettings.LatencyMode = GCLatencyMode.Batch; +GC.Collect(2, GCCollectionMode.Optimized); +``` + +## Immediate Actions (Priority Order) + +1. **Enable GC monitoring** - Add memory usage logging to identify hotspots +2. **Implement cache size limits** - Prevent unbounded growth of in-memory caches +3. **Use object pooling** - For frequently allocated objects like Song/Album/Artist +4. **Stream large responses** - Instead of loading entire JSON responses into memory +5. **Optimize JSON serialization** - Use source generators and reduce Dictionary usage + +## Monitoring Recommendations + +```csharp +// Add memory monitoring to Program.cs +builder.Services.AddHostedService(); + +public class MemoryMonitoringService : BackgroundService +{ + protected override async Task ExecuteAsync(CancellationToken stoppingToken) + { + while (!stoppingToken.IsCancellationRequested) + { + var memoryUsage = GC.GetTotalMemory(false); + var gen0 = GC.CollectionCount(0); + var gen1 = GC.CollectionCount(1); + var gen2 = GC.CollectionCount(2); + + _logger.LogInformation("Memory: {Memory:N0} bytes, GC: Gen0={Gen0}, Gen1={Gen1}, Gen2={Gen2}", + memoryUsage, gen0, gen1, gen2); + + await Task.Delay(TimeSpan.FromMinutes(5), stoppingToken); + } + } +} +``` \ No newline at end of file diff --git a/allstarr/Controllers/JellyfinController.cs b/allstarr/Controllers/JellyfinController.cs index b102518..bf15e4c 100644 --- a/allstarr/Controllers/JellyfinController.cs +++ b/allstarr/Controllers/JellyfinController.cs @@ -3460,6 +3460,13 @@ public class JellyfinController : ControllerBase { try { + // Check if already favorited (persistent tracking) + if (await IsTrackFavoritedAsync(itemId)) + { + _logger.LogInformation("Track already favorited (persistent): {ItemId}", itemId); + return; + } + // Get the song metadata first to build paths var song = await _metadataService.GetSongAsync(provider, externalId); if (song == null) @@ -3481,6 +3488,8 @@ public class JellyfinController : ControllerBase if (existingFiles.Length > 0) { _logger.LogInformation("Track already exists in kept folder: {Path}", existingFiles[0]); + // Mark as favorited even if we didn't download it + await MarkTrackAsFavoritedAsync(itemId, song); return; } } @@ -3529,6 +3538,7 @@ public class JellyfinController : ControllerBase if (System.IO.File.Exists(keptFilePath)) { _logger.LogInformation("Track already exists in kept folder (race condition): {Path}", keptFilePath); + await MarkTrackAsFavoritedAsync(itemId, song); return; } @@ -3546,6 +3556,9 @@ public class JellyfinController : ControllerBase _logger.LogDebug("Copied cover art to kept folder"); } } + + // Mark as favorited in persistent storage + await MarkTrackAsFavoritedAsync(itemId, song); } catch (Exception ex) { @@ -3560,63 +3573,241 @@ public class JellyfinController : ControllerBase { try { - // Get the song metadata to build paths - var song = await _metadataService.GetSongAsync(provider, externalId); - if (song == null) + // Mark for deletion instead of immediate deletion + await MarkTrackForDeletionAsync(itemId); + _logger.LogInformation("✓ Marked track for deletion: {ItemId}", itemId); + } + catch (Exception ex) + { + _logger.LogError(ex, "Error marking external track {ItemId} for deletion", itemId); + } + } + + #region Persistent Favorites Tracking + + private readonly string _favoritesFilePath = "/app/cache/favorites.json"; + + /// + /// Checks if a track is already favorited (persistent across restarts). + /// + private async Task IsTrackFavoritedAsync(string itemId) + { + try + { + if (!System.IO.File.Exists(_favoritesFilePath)) + return false; + + var json = await System.IO.File.ReadAllTextAsync(_favoritesFilePath); + var favorites = JsonSerializer.Deserialize>(json) ?? new(); + + return favorites.ContainsKey(itemId); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Failed to check favorite status for {ItemId}", itemId); + return false; + } + } + + /// + /// Marks a track as favorited in persistent storage. + /// + private async Task MarkTrackAsFavoritedAsync(string itemId, Song song) + { + try + { + var favorites = new Dictionary(); + + if (System.IO.File.Exists(_favoritesFilePath)) { - _logger.LogWarning("Could not find song metadata for {ItemId}", itemId); - return; + var json = await System.IO.File.ReadAllTextAsync(_favoritesFilePath); + favorites = JsonSerializer.Deserialize>(json) ?? new(); } - // Build kept folder path: /app/kept/Artist/Album/ + favorites[itemId] = new FavoriteTrackInfo + { + ItemId = itemId, + Title = song.Title, + Artist = song.Artist, + Album = song.Album, + FavoritedAt = DateTime.UtcNow + }; + + // Ensure cache directory exists + Directory.CreateDirectory(Path.GetDirectoryName(_favoritesFilePath)!); + + var updatedJson = JsonSerializer.Serialize(favorites, new JsonSerializerOptions { WriteIndented = true }); + await System.IO.File.WriteAllTextAsync(_favoritesFilePath, updatedJson); + + _logger.LogDebug("Marked track as favorited: {ItemId}", itemId); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Failed to mark track as favorited: {ItemId}", itemId); + } + } + + /// + /// Removes a track from persistent favorites storage. + /// + private async Task UnmarkTrackAsFavoritedAsync(string itemId) + { + try + { + if (!System.IO.File.Exists(_favoritesFilePath)) + return; + + var json = await System.IO.File.ReadAllTextAsync(_favoritesFilePath); + var favorites = JsonSerializer.Deserialize>(json) ?? new(); + + if (favorites.Remove(itemId)) + { + var updatedJson = JsonSerializer.Serialize(favorites, new JsonSerializerOptions { WriteIndented = true }); + await System.IO.File.WriteAllTextAsync(_favoritesFilePath, updatedJson); + _logger.LogDebug("Removed track from favorites: {ItemId}", itemId); + } + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Failed to remove track from favorites: {ItemId}", itemId); + } + } + + /// + /// Marks a track for deletion (delayed deletion for safety). + /// + private async Task MarkTrackForDeletionAsync(string itemId) + { + try + { + var deletionFilePath = "/app/cache/pending_deletions.json"; + var pendingDeletions = new Dictionary(); + + if (System.IO.File.Exists(deletionFilePath)) + { + var json = await System.IO.File.ReadAllTextAsync(deletionFilePath); + pendingDeletions = JsonSerializer.Deserialize>(json) ?? new(); + } + + // Mark for deletion 24 hours from now + pendingDeletions[itemId] = DateTime.UtcNow.AddHours(24); + + // Ensure cache directory exists + Directory.CreateDirectory(Path.GetDirectoryName(deletionFilePath)!); + + var updatedJson = JsonSerializer.Serialize(pendingDeletions, new JsonSerializerOptions { WriteIndented = true }); + await System.IO.File.WriteAllTextAsync(deletionFilePath, updatedJson); + + // Also remove from favorites immediately + await UnmarkTrackAsFavoritedAsync(itemId); + + _logger.LogDebug("Marked track for deletion in 24 hours: {ItemId}", itemId); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Failed to mark track for deletion: {ItemId}", itemId); + } + } + + /// + /// Information about a favorited track for persistent storage. + /// + private class FavoriteTrackInfo + { + public string ItemId { get; set; } = ""; + public string Title { get; set; } = ""; + public string Artist { get; set; } = ""; + public string Album { get; set; } = ""; + public DateTime FavoritedAt { get; set; } + } + + /// + /// Processes pending deletions (called by cleanup service). + /// + public async Task ProcessPendingDeletionsAsync() + { + try + { + var deletionFilePath = "/app/cache/pending_deletions.json"; + if (!System.IO.File.Exists(deletionFilePath)) + return; + + var json = await System.IO.File.ReadAllTextAsync(deletionFilePath); + var pendingDeletions = JsonSerializer.Deserialize>(json) ?? new(); + + var now = DateTime.UtcNow; + var toDelete = pendingDeletions.Where(kvp => kvp.Value <= now).ToList(); + var remaining = pendingDeletions.Where(kvp => kvp.Value > now).ToDictionary(kvp => kvp.Key, kvp => kvp.Value); + + foreach (var (itemId, _) in toDelete) + { + await ActuallyDeleteTrackAsync(itemId); + } + + if (toDelete.Count > 0) + { + // Update pending deletions file + var updatedJson = JsonSerializer.Serialize(remaining, new JsonSerializerOptions { WriteIndented = true }); + await System.IO.File.WriteAllTextAsync(deletionFilePath, updatedJson); + + _logger.LogInformation("Processed {Count} pending deletions", toDelete.Count); + } + } + catch (Exception ex) + { + _logger.LogError(ex, "Error processing pending deletions"); + } + } + + /// + /// Actually deletes a track from the kept folder. + /// + private async Task ActuallyDeleteTrackAsync(string itemId) + { + try + { + var (isExternal, provider, externalId) = _localLibraryService.ParseSongId(itemId); + if (!isExternal) return; + + var song = await _metadataService.GetSongAsync(provider!, externalId!); + if (song == null) return; + var keptBasePath = "/app/kept"; var keptArtistPath = Path.Combine(keptBasePath, PathHelper.SanitizeFileName(song.Artist)); var keptAlbumPath = Path.Combine(keptArtistPath, PathHelper.SanitizeFileName(song.Album)); - if (!Directory.Exists(keptAlbumPath)) - { - _logger.LogInformation("Track not in kept folder (album folder doesn't exist): {ItemId}", itemId); - return; - } + if (!Directory.Exists(keptAlbumPath)) return; - // Find and remove the track file var sanitizedTitle = PathHelper.SanitizeFileName(song.Title); var trackFiles = Directory.GetFiles(keptAlbumPath, $"*{sanitizedTitle}*"); - if (trackFiles.Length == 0) - { - _logger.LogInformation("Track not found in kept folder: {ItemId}", itemId); - return; - } - foreach (var trackFile in trackFiles) { System.IO.File.Delete(trackFile); - _logger.LogInformation("✓ Removed track from kept folder: {Path}", trackFile); + _logger.LogInformation("✓ Deleted track from kept folder: {Path}", trackFile); } // Clean up empty directories if (Directory.GetFiles(keptAlbumPath).Length == 0 && Directory.GetDirectories(keptAlbumPath).Length == 0) { Directory.Delete(keptAlbumPath); - _logger.LogDebug("Removed empty album folder: {Path}", keptAlbumPath); - // Also remove artist folder if empty if (Directory.Exists(keptArtistPath) && Directory.GetFiles(keptArtistPath).Length == 0 && Directory.GetDirectories(keptArtistPath).Length == 0) { Directory.Delete(keptArtistPath); - _logger.LogDebug("Removed empty artist folder: {Path}", keptArtistPath); } } } catch (Exception ex) { - _logger.LogError(ex, "Error removing external track {ItemId} from kept folder", itemId); + _logger.LogWarning(ex, "Failed to delete track {ItemId}", itemId); } } + #endregion + /// /// Loads missing tracks from file cache as fallback when Redis is empty. /// diff --git a/allstarr/Program.cs b/allstarr/Program.cs index 1d37077..fdb7d3d 100644 --- a/allstarr/Program.cs +++ b/allstarr/Program.cs @@ -388,6 +388,9 @@ if (backendType == BackendType.Jellyfin) builder.Services.AddSingleton(); builder.Services.AddScoped(); builder.Services.AddScoped(); + + // Register JellyfinController as a service for dependency injection + builder.Services.AddScoped(); } else { diff --git a/allstarr/Services/Common/CacheCleanupService.cs b/allstarr/Services/Common/CacheCleanupService.cs index 218f5b0..e62e71a 100644 --- a/allstarr/Services/Common/CacheCleanupService.cs +++ b/allstarr/Services/Common/CacheCleanupService.cs @@ -1,5 +1,6 @@ using Microsoft.Extensions.Options; using allstarr.Models.Settings; +using allstarr.Controllers; namespace allstarr.Services.Common; @@ -11,16 +12,19 @@ public class CacheCleanupService : BackgroundService { private readonly IConfiguration _configuration; private readonly SubsonicSettings _subsonicSettings; + private readonly IServiceProvider _serviceProvider; private readonly ILogger _logger; private readonly TimeSpan _cleanupInterval = TimeSpan.FromHours(1); public CacheCleanupService( IConfiguration configuration, IOptions subsonicSettings, + IServiceProvider serviceProvider, ILogger logger) { _configuration = configuration; _subsonicSettings = subsonicSettings.Value; + _serviceProvider = serviceProvider; _logger = logger; } @@ -41,6 +45,7 @@ public class CacheCleanupService : BackgroundService try { await CleanupOldCachedFilesAsync(stoppingToken); + await ProcessPendingDeletionsAsync(stoppingToken); await Task.Delay(_cleanupInterval, stoppingToken); } catch (OperationCanceledException) @@ -160,4 +165,30 @@ public class CacheCleanupService : BackgroundService await Task.CompletedTask; } + + /// + /// Processes pending track deletions from the kept folder. + /// + private async Task ProcessPendingDeletionsAsync(CancellationToken cancellationToken) + { + try + { + // Create a scope to get the JellyfinController + using var scope = _serviceProvider.CreateScope(); + var jellyfinController = scope.ServiceProvider.GetService(); + + if (jellyfinController != null) + { + await jellyfinController.ProcessPendingDeletionsAsync(); + } + else + { + _logger.LogWarning("Could not resolve JellyfinController for pending deletions processing"); + } + } + catch (Exception ex) + { + _logger.LogError(ex, "Error processing pending deletions"); + } + } }