Skip to content
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,22 @@ public void Unload(IPerformanceBudget frameTimeBudget, int maxUnloadAmount)
ProfilingCounters.GltfInCacheAmount.Value -= unloadedAmount;
}

public void Remove(in string key)
{
if (cache.TryGetValue(key, out List<GltfContainerAsset> assets))
{
foreach (GltfContainerAsset asset in assets)
asset.Dispose();

ProfilingCounters.GltfInCacheAmount.Value -= assets.Count;
assets.Clear();
cache.Remove(key);
unloadQueue.TryRemove(key);
}

IrrecoverableFailures.Remove(key);
}

bool IEqualityComparer<string>.Equals(string x, string y) =>
string.Equals(x, y, StringComparison.OrdinalIgnoreCase);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ public interface IGltfContainerAssetsCache

void Unload(IPerformanceBudget frameTimeBudget, int maxUnloadAmount);

/// <summary>
/// Evict a single cache key, disposing every pooled asset under it. Lets an edited GLTF be
/// dropped in isolation while the rest of the cache stays warm across a scene reload.
/// </summary>
void Remove(in string key);

void Dereference(in string key, GltfContainerAsset asset, bool putInBridge = false, bool handleAssetLoad = true);

void SetAssetLoadCache(AssetPreLoadCache assetPreLoadCache);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ public interface IStreamableCache<TAsset, TLoadingIntention> : IDisposable where
/// </summary>
void Unload(IPerformanceBudget frameTimeBudget, int maxUnloadAmount);

/// <summary>
/// Evict a single entry by key, disposing its asset when it is no longer referenced.
/// Mirrors <see cref="Unload" /> scoped to one key, so an edited asset can be dropped
/// without draining the whole cache. Default is a no-op for caches that hold nothing.
/// </summary>
void Remove(in TLoadingIntention key) { }

#region Referencing
/// <summary>
/// Base implementation is empty as not every asset requires reference counting
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,5 +79,23 @@ public void Unload(IPerformanceBudget frameTimeBudget, int maxUnloadAmount)

inCacheCount.Value = cache.Count;
}

public void Remove(in TLoadingIntention key)
{
if (!cache.TryGetValue(key, out TAssetData asset) || !asset.CanBeDisposed())
return;

asset.Dispose();
cache.Remove(key);

for (int i = listedCache.Count - 1; i >= 0; i--)
if (IntentionsComparer<TLoadingIntention>.INSTANCE.Equals(listedCache[i].intention, key))
{
listedCache.RemoveAt(i);
break;
}

inCacheCount.Value = cache.Count;
Comment thread
dalkia marked this conversation as resolved.
}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using Arch.Core;
using Cysharp.Threading.Tasks;
using DCL.Character.Components;
using DCL.Ipfs;
using DCL.ResourcesUnloading;
using ECS.LifeCycle.Components;
using ECS.SceneLifeCycle.Components;
Expand All @@ -14,6 +15,22 @@

namespace ECS.SceneLifeCycle
{
/// <summary>
/// The single GLTF model a local-development hot reload reported as changed, carried from the
/// dev server's websocket message so the reload can evict just that asset instead of the whole cache.
/// </summary>
public readonly struct ChangedGltfModel
{
public readonly string Src;
public readonly string Hash;

public ChangedGltfModel(string src, string hash)
{
Src = src;
Hash = hash;
}
}

public class ECSReloadScene
{
private readonly IScenesCache scenesCache;
Expand Down Expand Up @@ -44,19 +61,19 @@ public ECSReloadScene(IScenesCache scenesCache,
var foundEntity = FindSceneEntity(sceneInCache);
if (foundEntity == Entity.Null) return null;

await DisposeAndRestartAsync(foundEntity, sceneInCache, ct);
await DisposeAndRestartAsync(foundEntity, sceneInCache, null, ct);

return sceneInCache;
}

public async UniTask<ISceneFacade?> TryReloadSceneAsync(CancellationToken ct, string sceneId)
public async UniTask<ISceneFacade?> TryReloadSceneAsync(CancellationToken ct, string sceneId, ChangedGltfModel? changedModel = null)
{
if (!scenesCache.TryGetBySceneId(sceneId, out var sceneInCache)) return null;

var foundEntity = FindSceneEntity(sceneInCache!);
if (foundEntity == Entity.Null) return null;

await DisposeAndRestartAsync(foundEntity, sceneInCache!, ct);
await DisposeAndRestartAsync(foundEntity, sceneInCache!, changedModel, ct);

return sceneInCache;
}
Expand All @@ -74,10 +91,15 @@ private Entity FindSceneEntity(ISceneFacade targetScene)
return sceneEntity;
}

private async UniTask DisposeAndRestartAsync(Entity entity, ISceneFacade currentScene, CancellationToken ct)
private async UniTask DisposeAndRestartAsync(Entity entity, ISceneFacade currentScene, ChangedGltfModel? changedModel, CancellationToken ct)
{
ct.ThrowIfCancellationRequested();

// Captured before the teardown below strips the scene entity's components.
SceneEntityDefinition? definition = localSceneDevelopment
? world.Get<SceneDefinitionComponent>(entity).Definition
: null;

//There is a lingering promise we need to remove, and add the DeleteEntityIntention to make the standard unload flow.
world.Add<DeleteEntityIntention>(entity);

Expand All @@ -96,11 +118,21 @@ private async UniTask DisposeAndRestartAsync(Entity entity, ISceneFacade current
world.Query(in new QueryDescription().WithAll<RealmComponent>(),
(ref StaticScenePointers staticScenePointers) => { staticScenePointers.Promise = null; });

// Force-drain dereferenced caches on LSD reload. The local dev server derives hashes
// from the file path, not content, so an updated model keeps the same hash and cache
// hits would return stale assets. Draining guarantees fresh loads.
cacheCleaner.UnloadCache(budgeted: false);
Resources.UnloadUnusedAssets();
if (changedModel is { } model && IsRawGltfModel(definition, model.Hash))
{
// The dev server named the exact model that changed. In raw-GLTF development its
// cache key is the bare content hash, so evict just that asset and let every other
// cache stay warm across the reload.
cacheCleaner.EvictGltfModel(model.Hash, model.Src);
}
else
{
// Force-drain dereferenced caches on LSD reload. The local dev server derives hashes
// from the file path, not content, so an updated model keeps the same hash and cache
// hits would return stale assets. Draining guarantees fresh loads.
cacheCleaner.UnloadCache(budgeted: false);
Resources.UnloadUnusedAssets();
}

await WaitUntilNewSceneIsFullyLoadedAsync();
}
Expand Down Expand Up @@ -134,5 +166,19 @@ await UniTask.WaitUntil(() =>
}, cancellationToken: ct);
}
}

/// <summary>
/// True when the hash addresses a raw GLTF, i.e. no asset-bundle manifest maps it. The GLTF
/// container cache is keyed by <see cref="AssetBundleManifestVersion.ComposeCacheKey" />,
/// which returns the bare hash only in that case; under <c>--local-ab</c> the key differs and
/// the model lives in the asset-bundle caches instead, so scoped eviction must not be used.
/// </summary>
internal static bool IsRawGltfModel(SceneEntityDefinition? definition, string hash)
Comment thread
dalkia marked this conversation as resolved.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Nullability annotation (repeat from previous review) — hash is declared string (non-nullable) but is null-checked via string.IsNullOrEmpty. Per CLAUDE.md anti-patterns: "Defensive null-checks against non-null declarations — if the declared type is T (not T?), don't null-check it." Since null is a legitimate defensive case (the test in ECSReloadSceneShould exercises it), the parameter should honestly declare string?. This also lets the test pass null directly instead of null!.

Suggested change
internal static bool IsRawGltfModel(SceneEntityDefinition? definition, string hash)
internal static bool IsRawGltfModel(SceneEntityDefinition? definition, string? hash)

{
if (definition == null || string.IsNullOrEmpty(hash))
return false;

return definition.AssetBundleManifestVersionOrFailed.ComposeCacheKey(hash) == hash;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,21 @@ private async UniTask ConnectToServerAsync(string localSceneWebsocketServer,
wsSceneMessage.MergeFrom(receiveBuffer.AsSpan(0, receiveResult.Count));
ReportHub.Log(ReportCategory.SDK_LOCAL_SCENE_DEVELOPMENT, $"Websocket scene message received: {wsSceneMessage.MessageCase}");

// TODO: Discriminate 'wsSceneMessage.MessageCase == WsSceneMessage.MessageOneofCase.UpdateModel' to only update GLTF models...
// An UpdateModel message names the single GLTF that changed; carry it through so the
// reload can evict just that asset instead of draining every cache.
string sceneId;
ChangedGltfModel? changedModel;

if (wsSceneMessage.MessageCase == WsSceneMessage.MessageOneofCase.UpdateModel)
{
sceneId = wsSceneMessage.UpdateModel.SceneId;
changedModel = new ChangedGltfModel(wsSceneMessage.UpdateModel.Src, wsSceneMessage.UpdateModel.Hash);
}
else
{
sceneId = wsSceneMessage.UpdateScene.SceneId;
changedModel = null;
}

// Switch to the main thread because `TryReloadSceneAsync` requires that
await UniTask.SwitchToMainThread(cancellationToken: ct);
Expand All @@ -86,8 +100,7 @@ private async UniTask ConnectToServerAsync(string localSceneWebsocketServer,
// And pause the skybox update while loading to avoid transitions
globalWorld.AddOrGet(skyboxEntity, new PauseSkyboxTimeUpdate());

await reloadScene.TryReloadSceneAsync(ct,
wsSceneMessage.MessageCase == WsSceneMessage.MessageOneofCase.UpdateScene ? wsSceneMessage.UpdateScene.SceneId : wsSceneMessage.UpdateModel.SceneId)
await reloadScene.TryReloadSceneAsync(ct, sceneId, changedModel)
.Timeout(TimeSpan.FromSeconds(RELOAD_SCENE_TIMEOUT_SECS));
}
catch (TimeoutException) { }
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
using DCL.Ipfs;
using ECS.SceneLifeCycle;
using NUnit.Framework;
using System;

namespace DCL.SceneLifeCycle.Tests
{
public class ECSReloadSceneShould
{
[Test]
public void TreatDefinitionWithoutManifestAsRawGltf()
{
//Arrange: no asset-bundle manifest -> the container cache is keyed by the bare hash
SceneEntityDefinition definition = CreateDefinition();

//Act & Assert
Assert.That(ECSReloadScene.IsRawGltfModel(definition, "b64-somehash"), Is.True);
}

[Test]
public void NotTreatMissingDefinitionOrEmptyHashAsRawGltf()
{
SceneEntityDefinition definition = CreateDefinition();

Assert.That(ECSReloadScene.IsRawGltfModel(null, "b64-somehash"), Is.False);
Assert.That(ECSReloadScene.IsRawGltfModel(definition, string.Empty), Is.False);
Assert.That(ECSReloadScene.IsRawGltfModel(definition, null!), Is.False);
}

private static SceneEntityDefinition CreateDefinition() =>
new ("test-scene", new SceneMetadata())
{
pointers = new[] { "0,0" },
content = Array.Empty<ContentDefinition>(),
};
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,8 @@ public void Dereference(in string key, GltfContainerAsset asset, bool putInBridg

public void Unload(IPerformanceBudget frameTimeBudget, int maxUnloadAmount) { }

public void Remove(in string key) { }

public void SetAssetLoadCache(AssetPreLoadCache assetPreLoadCache) { }
}
}
Expand Down
9 changes: 9 additions & 0 deletions Explorer/Assets/DCL/ResourcesUnloading/CacheCleaner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,15 @@ public void UnloadCache(bool budgeted = true)
ClearExtendedObjectPools(budgetToUse, budgeted ? POOLS_UNLOAD_CHUNK : int.MaxValue);
}

public void EvictGltfModel(string hash, string src)
{
// In raw-GLTF development the container cache is keyed by the bare content hash, while the
// parsed-import cache is keyed by (src, hash). Evict both: dropping only the container asset
// would let it be rebuilt from the still-cached stale import.
gltfContainerAssetsCache?.Remove(hash);
gltfLoadCache?.Remove(GetGLTFIntention.Create(src, hash));
}

private void ClearExtendedObjectPools(IPerformanceBudget budgetToUse, int maxUnload)
{
foreach (IThrottledClearable pool in extendedObjectPools)
Expand Down
8 changes: 8 additions & 0 deletions Explorer/Assets/DCL/ResourcesUnloading/ICacheCleaner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@
public interface ICacheCleaner
{
void UnloadCache(bool budgeted = true);

/// <summary>
/// Evict a single raw-GLTF model (parsed import and instantiated container asset) by its
/// content hash, leaving every other cache warm. Used on a scene reload when the dev server
/// told us exactly which model changed, so we can avoid draining the whole cache.
/// </summary>
void EvictGltfModel(string hash, string src);

void UpdateProfilingCounters();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,46 @@ public void ShouldCleanCachesWithRespectToReferencing()
Assert.That(innerOfCache.Count, Is.EqualTo(0));
}

[Category(INTEGRATION)]
[Test]
public void EvictGltfModelDropsTargetedModelAndKeepsOtherCachesWarm()
{
// Arrange
FillCachesWithElements(hashID: "test");

Assert.That(gltfContainerAssetsCache.cache.Count, Is.EqualTo(1));
Assert.That(texturesCache.cache.Count, Is.EqualTo(1));
Assert.That(audioClipsCache.cache.Count, Is.EqualTo(1));

// Act
cacheCleaner.EvictGltfModel("test", "model.glb");

// Assert: only the GLTF container entry for that hash is gone; unrelated caches stay warm
Assert.That(gltfContainerAssetsCache.cache.Count, Is.EqualTo(0));
Assert.That(texturesCache.cache.Count, Is.EqualTo(1));
Assert.That(audioClipsCache.cache.Count, Is.EqualTo(1));
Assert.That(assetBundleCache.cache.Count, Is.EqualTo(1));
}

[Category(INTEGRATION)]
[Test]
public void RemoveEvictsSingleStreamableEntryLeavingOthers()
{
// Arrange
var keyA = new GetTextureIntention { CommonArguments = new CommonLoadingArguments { URL = URLAddress.FromString("textureA") } };
var keyB = new GetTextureIntention { CommonArguments = new CommonLoadingArguments { URL = URLAddress.FromString("textureB") } };
texturesCache.Add(keyA, new TextureData(new Texture2D(1, 1)));
texturesCache.Add(keyB, new TextureData(new Texture2D(1, 1)));

// Act
texturesCache.Remove(keyA);

// Assert
Assert.That(texturesCache.cache.Count, Is.EqualTo(1));
Assert.That(texturesCache.TryGet(keyB, out _), Is.True);
Assert.That(texturesCache.TryGet(keyA, out _), Is.False);
}

private void FillCachesWithElements(string hashID)
{
var textureIntention = new GetTextureIntention { CommonArguments = new CommonLoadingArguments { URL = URLAddress.FromString(hashID) } };
Expand Down
Loading