Skip to content

Performance: Avoid allocating a string if _publishedContentCache has a cached version in MediaCacheService #22535

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
AndyButland merged 2 commits into from
Apr 20, 2026
+14 −21
Merged

Performance: Avoid allocating a string if _publishedContentCache has a cached version in MediaCacheService #22535

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2772616
Avoid allocating a string if _publishedContentCache has a cached vers…
Henr1k80 Apr 20, 2026
9cca8d4
Clarified comment, used GetCacheKey method from location where string…
AndyButland Apr 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 14 additions & 21 deletions src/Umbraco.PublishedCache.HybridCache/Services/MediaCacheService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#if DEBUG

Check notice on line 1 in src/Umbraco.PublishedCache.HybridCache/Services/MediaCacheService.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

✅ No longer an issue: Primitive Obsession 

The ratio of primivite types in function arguments is no longer above the threshold
using System.Diagnostics;
#endif
using System.Collections.Concurrent;
Expand Down Expand Up @@ -32,7 +32,7 @@
private readonly ILogger<MediaCacheService> _logger;
private readonly CacheSettings _cacheSettings;

private readonly ConcurrentDictionary<string, IPublishedContent> _publishedContentCache = [];
private readonly ConcurrentDictionary<Guid, IPublishedContent> _publishedContentCache = [];

private HashSet<Guid>? _seedKeys;
private HashSet<Guid> SeedKeys
Expand Down Expand Up @@ -105,13 +105,12 @@

private async Task<IPublishedContent?> GetNodeAsync(Guid key)
{
var cacheKey = $"{key}";

if (_publishedContentCache.TryGetValue(cacheKey, out IPublishedContent? cached))
if (_publishedContentCache.TryGetValue(key, out IPublishedContent? cached))
{
return cached;
}

string cacheKey = GetCacheKey(key);
(bool exists, ContentCacheNode? contentCacheNode) = await _hybridCache.TryGetValueAsync<ContentCacheNode?>(cacheKey, CancellationToken.None);
if (exists is false)
{
Expand All @@ -135,7 +134,7 @@
IPublishedContent? result = _publishedContentFactory.ToIPublishedMedia(contentCacheNode).CreateModel(_publishedModelFactory);
if (result is not null)
{
_publishedContentCache[cacheKey] = result;
_publishedContentCache[key] = result;
}

return result;
Expand All @@ -157,7 +156,7 @@
return false;
}

return await _hybridCache.ExistsAsync<ContentCacheNode?>($"{keyAttempt.Result}", CancellationToken.None);
return await _hybridCache.ExistsAsync<ContentCacheNode?>(GetCacheKey(keyAttempt.Result), CancellationToken.None);
}

public async Task RefreshMediaAsync(IMedia media)
Expand All @@ -174,7 +173,7 @@

var cacheNode = _cacheNodeFactory.ToContentCacheNode(media);
await _databaseCacheRepository.RefreshMediaAsync(cacheNode);
_publishedContentCache.Remove(GetCacheKey(media.Key, false), out _);
_publishedContentCache.Remove(media.Key, out _);
scope.Complete();
}

Expand Down Expand Up @@ -202,9 +201,7 @@
break;
}

var cacheKey = GetCacheKey(key, false);

var existsInCache = await _hybridCache.ExistsAsync<ContentCacheNode?>(cacheKey, CancellationToken.None);
var existsInCache = await _hybridCache.ExistsAsync<ContentCacheNode?>(GetCacheKey(key), CancellationToken.None);
if (existsInCache is false)
{
uncachedKeys.Add(key);
Expand All @@ -228,9 +225,8 @@

foreach (ContentCacheNode cacheNode in cacheNodes)
{
var cacheKey = GetCacheKey(cacheNode.Key, false);
await _hybridCache.SetAsync(
cacheKey,
GetCacheKey(cacheNode.Key),
cacheNode,
GetSeedEntryOptions(),
GenerateTags(cacheNode),
Expand All @@ -253,9 +249,8 @@
ContentCacheNode? publishedNode = await _databaseCacheRepository.GetMediaSourceAsync(key);
if (publishedNode is not null)
{
var cacheKey = GetCacheKey(publishedNode.Key, false);
await _hybridCache.SetAsync(cacheKey, publishedNode, GetEntryOptions(publishedNode.Key));
_publishedContentCache.Remove(cacheKey, out _);
await _hybridCache.SetAsync(GetCacheKey(publishedNode.Key), publishedNode, GetEntryOptions(publishedNode.Key));
_publishedContentCache.Remove(key, out _);
}
else
{
Expand Down Expand Up @@ -335,18 +330,16 @@
LocalCacheExpiration = _cacheSettings.Entry.Media.SeedCacheDuration,
};

private static string GetCacheKey(Guid key, bool preview) => preview ? $"{key}+draft" : $"{key}";
private static string GetCacheKey(Guid key) => $"{key}";

// Generates the cache tags for a given CacheNode
// Generates the cache tags for a given CacheNode.
// We use the tags to be able to clear all cache entries that are related to a given content item.
// Tags for now are only content/media, but can be expanded with draft/published later.
private static HashSet<string> GenerateTags(ContentCacheNode? cacheNode) => cacheNode is null ? [] : [Constants.Cache.Tags.Media, MediaTypeIdTag(cacheNode.ContentTypeId)];

private async Task ClearPublishedCacheAsync(Guid key)
{
var cacheKey = GetCacheKey(key, false);
await _hybridCache.RemoveAsync(cacheKey);
_publishedContentCache.Remove(cacheKey, out _);
await _hybridCache.RemoveAsync(GetCacheKey(key));
_publishedContentCache.Remove(key, out _);
}

private static string MediaTypeIdTag(int mediaTypeId)
Expand Down
Loading