Skip to content

Commit

Permalink
Make MutableAppearance disposable, allowing for pooling
Browse files Browse the repository at this point in the history
Along with related memory optimizations. Saves >1GB of allocations on Paradise init.
  • Loading branch information
wixoaGit committed Jan 3, 2025
1 parent 732301b commit 4b548c5
Show file tree
Hide file tree
Showing 9 changed files with 103 additions and 80 deletions.
2 changes: 0 additions & 2 deletions OpenDreamClient/Rendering/DreamIcon.cs
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,6 @@ private void UpdateAnimation() {
ColorMatrix.Interpolate(in _appearance.ColorMatrix, in endAppearance.ColorMatrix, factor, out _animatedAppearance.ColorMatrix);
}


if (endAppearance.GlideSize != _appearance.GlideSize) {
_animatedAppearance.GlideSize = ((1-factor) * _appearance.GlideSize) + (factor * endAppearance.GlideSize);
}
Expand Down Expand Up @@ -444,7 +443,6 @@ private void UpdateAnimation() {
AppearanceAnimation repeatAnimation = new AppearanceAnimation(start, animation.Duration, animation.EndAppearance, animation.Easing, animation.Flags, animation.Delay, animation.LastInSequence);
toReAdd.Add(repeatAnimation);
}

}
}

Expand Down
15 changes: 7 additions & 8 deletions OpenDreamRuntime/AtomManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ public bool TryGetAppearance(DreamObject atom, [NotNullWhen(true)] out Immutable

public void UpdateAppearance(DreamObject atom, Action<MutableAppearance> update) {
ImmutableAppearance immutableAppearance = MustGetAppearance(atom);
MutableAppearance appearance = immutableAppearance.ToMutable(); // Clone the appearance
using var appearance = immutableAppearance.ToMutable(); // Clone the appearance
update(appearance);
SetAtomAppearance(atom, appearance);
}
Expand All @@ -540,7 +540,7 @@ public void SetAtomAppearance(DreamObject atom, MutableAppearance appearance) {
DMISpriteSystem.SetSpriteAppearance(new(movable.Entity, movable.SpriteComponent), appearance);
} else if (atom is DreamObjectImage image) {
if(image.IsMutableAppearance)
image.MutableAppearance = new(appearance); //this needs to be a copy
image.MutableAppearance = MutableAppearance.GetCopy(appearance); //this needs to be a copy
else
DMISpriteSystem.SetSpriteAppearance(new(image.Entity, image.SpriteComponent!), appearance);
} else if (atom is DreamObjectArea area) {
Expand Down Expand Up @@ -606,7 +606,7 @@ public void AnimateAppearance(DreamObject atom, TimeSpan duration, AnimationEasi

public bool TryCreateAppearanceFrom(DreamValue value, [NotNullWhen(true)] out MutableAppearance? appearance) {
if (value.TryGetValueAsAppearance(out var copyFromAppearance)) {
appearance = new(copyFromAppearance);
appearance = MutableAppearance.GetCopy(copyFromAppearance);
return true;
}

Expand All @@ -616,7 +616,7 @@ public bool TryCreateAppearanceFrom(DreamValue value, [NotNullWhen(true)] out Mu
}

if (value.TryGetValueAsType(out var copyFromType)) {
appearance = new(GetAppearanceFromDefinition(copyFromType.ObjectDefinition));
appearance = MutableAppearance.GetCopy(GetAppearanceFromDefinition(copyFromType.ObjectDefinition));
return true;
}

Expand All @@ -626,9 +626,8 @@ public bool TryCreateAppearanceFrom(DreamValue value, [NotNullWhen(true)] out Mu
}

if (_resourceManager.TryLoadIcon(value, out var iconResource)) {
appearance = new MutableAppearance() {
Icon = iconResource.Id
};
appearance = MutableAppearance.Get();
appearance.Icon = iconResource.Id;

return true;
}
Expand Down Expand Up @@ -660,7 +659,7 @@ public MutableAppearance GetAppearanceFromDefinition(DreamObjectDefinition def)
def.TryGetVariable("blend_mode", out var blendModeVar);
def.TryGetVariable("appearance_flags", out var appearanceFlagsVar);

appearance = new MutableAppearance();
appearance = MutableAppearance.Get();
SetAppearanceVar(appearance, "name", nameVar);
SetAppearanceVar(appearance, "icon", iconVar);
SetAppearanceVar(appearance, "icon_state", stateVar);
Expand Down
2 changes: 1 addition & 1 deletion OpenDreamRuntime/DreamMapManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ public void SetTurfAppearance(DreamObjectTurf turf, MutableAppearance appearance
if(turf.Cell.Area.Appearance != _appearanceSystem.DefaultAppearance)
if(!appearance.Overlays.Contains(turf.Cell.Area.Appearance)) {
if(!_turfAreaLookup.TryGetValue((appearance, turf.Cell.Area.Appearance.MustGetID()), out var newAppearance)) {
newAppearance = new(appearance);
newAppearance = MutableAppearance.GetCopy(appearance);
newAppearance.Overlays.Add(turf.Cell.Area.Appearance);
_turfAreaLookup.Add((appearance, turf.Cell.Area.Appearance.MustGetID()), newAppearance);
}
Expand Down
16 changes: 8 additions & 8 deletions OpenDreamRuntime/Objects/Types/DreamList.cs
Original file line number Diff line number Diff line change
Expand Up @@ -695,8 +695,8 @@ public override DreamValue GetValue(DreamValue key) {
if (_appearanceSystem == null)
return DreamValue.Null;

ImmutableAppearance overlayAppearance = overlaysList[overlayIndex - 1];
return new DreamValue(overlayAppearance.ToMutable());
var overlayAppearance = overlaysList[overlayIndex - 1].ToMutable();
return new DreamValue(overlayAppearance);
}

public override void SetValue(DreamValue key, DreamValue value, bool allowGrowth = false) {
Expand All @@ -707,9 +707,9 @@ public override void AddValue(DreamValue value) {
if (_appearanceSystem == null)
return;

MutableAppearance? overlayAppearance = CreateOverlayAppearance(_atomManager, value, _atomManager.MustGetAppearance(_owner).Icon);
overlayAppearance ??= new MutableAppearance();
ImmutableAppearance immutableOverlay = _appearanceSystem.AddAppearance(overlayAppearance);
var overlayAppearance = CreateOverlayAppearance(_atomManager, value, _atomManager.MustGetAppearance(_owner).Icon);
var immutableOverlay = _appearanceSystem.AddAppearance(overlayAppearance ?? MutableAppearance.Default);
overlayAppearance?.Dispose();

//after UpdateApparance is done, the atom is set with a new immutable appearance containing a hard ref to the overlay
//only /mutable_appearance handles it differently, and that's done in DreamObjectImage
Expand All @@ -728,6 +728,7 @@ public override void RemoveValue(DreamValue value) {

_atomManager.UpdateAppearance(_owner, appearance => {
GetOverlaysList(appearance).Remove(_appearanceSystem.AddAppearance(overlayAppearance, registerAppearance:false));
overlayAppearance.Dispose();
});
}

Expand All @@ -747,9 +748,8 @@ private ImmutableAppearance[] GetOverlaysArray(ImmutableAppearance appearance) =
MutableAppearance overlay;

if (value.TryGetValueAsString(out var iconState)) {
overlay = new MutableAppearance() {
IconState = iconState
};
overlay = MutableAppearance.Get();
overlay.IconState = iconState;
overlay.Icon ??= defaultIcon;
} else if (atomManager.TryCreateAppearanceFrom(value, out var overlayAppearance)) {
overlay = overlayAppearance;
Expand Down
4 changes: 2 additions & 2 deletions OpenDreamRuntime/Objects/Types/DreamObjectAtom.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using OpenDreamRuntime.Procs;
using OpenDreamShared.Dream;

namespace OpenDreamRuntime.Objects.Types;

Expand Down Expand Up @@ -117,6 +116,7 @@ protected override void SetVar(string varName, DreamValue value) {
newAppearance.Direction = AtomManager.MustGetAppearance(this).Direction;

AtomManager.SetAtomAppearance(this, newAppearance);
newAppearance.Dispose();
break;
case "overlays": {
Overlays.Cut();
Expand Down Expand Up @@ -178,7 +178,7 @@ protected override void SetVar(string varName, DreamValue value) {
default:
if (AtomManager.IsValidAppearanceVar(varName)) {
// Basically AtomManager.UpdateAppearance() but without the performance impact of using actions
MutableAppearance appearance = AtomManager.MustGetAppearance(this).ToMutable();
using var appearance = AtomManager.MustGetAppearance(this).ToMutable();
AtomManager.SetAppearanceVar(appearance, varName, value);
AtomManager.SetAtomAppearance(this, appearance);
break;
Expand Down
9 changes: 7 additions & 2 deletions OpenDreamRuntime/Objects/Types/DreamObjectImage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ public override void Initialize(DreamProcArguments args) {
}

AtomManager.SetAtomAppearance(this, mutableAppearance);
mutableAppearance.Dispose();
}

protected override bool TryGetVar(string varName, out DreamValue value) {
Expand Down Expand Up @@ -117,6 +118,7 @@ protected override void SetVar(string varName, DreamValue value) {
var originalAppearance = AtomManager.MustGetAppearance(this);
newAppearance.Direction = originalAppearance.Direction;
AtomManager.SetAtomAppearance(this, newAppearance);
newAppearance.Dispose();
break;
case "loc":
value.TryGetValueAsDreamObject(out _loc);
Expand All @@ -138,6 +140,7 @@ protected override void SetVar(string varName, DreamValue value) {

_overlays.Cut();
_overlays.AddValue(new(overlay));
overlay.Dispose();
}

return;
Expand Down Expand Up @@ -170,6 +173,7 @@ protected override void SetVar(string varName, DreamValue value) {

_underlays.Cut();
_underlays.AddValue(new(underlay));
underlay.Dispose();
}

return;
Expand Down Expand Up @@ -206,14 +210,14 @@ protected override void SetVar(string varName, DreamValue value) {
break;
}
case "override": {
MutableAppearance mutableAppearance = IsMutableAppearance ? MutableAppearance! : AtomManager.MustGetAppearance(this).ToMutable();
using var mutableAppearance = IsMutableAppearance ? MutableAppearance! : AtomManager.MustGetAppearance(this).ToMutable();
mutableAppearance.Override = value.IsTruthy();
AtomManager.SetAtomAppearance(this, mutableAppearance);
break;
}
default:
if (AtomManager.IsValidAppearanceVar(varName)) {
MutableAppearance mutableAppearance = IsMutableAppearance ? MutableAppearance! : AtomManager.MustGetAppearance(this).ToMutable();
using var mutableAppearance = IsMutableAppearance ? MutableAppearance! : AtomManager.MustGetAppearance(this).ToMutable();
AtomManager.SetAppearanceVar(mutableAppearance, varName, value);
AtomManager.SetAtomAppearance(this, mutableAppearance);
break;
Expand All @@ -239,6 +243,7 @@ protected override void HandleDeletion(bool possiblyThreaded) {
EntityManager.DeleteEntity(Entity);
}

MutableAppearance?.Dispose();
base.HandleDeletion(possiblyThreaded);
}
}
2 changes: 1 addition & 1 deletion OpenDreamRuntime/Objects/Types/DreamObjectTurf.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public void OnAreaChange(DreamObjectArea oldArea) {
if (Cell == null!)
return;

var newAppearance = Appearance.ToMutable();
using var newAppearance = Appearance.ToMutable();

newAppearance.Overlays.Remove(oldArea.Appearance);
DreamMapManager.SetTurfAppearance(this, newAppearance);
Expand Down
77 changes: 37 additions & 40 deletions OpenDreamShared/Dream/ImmutableAppearance.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ namespace OpenDreamShared.Dream;

// TODO: Wow this is huge! Probably look into splitting this by most used/least used to reduce the size of these
[Serializable]
public sealed class ImmutableAppearance : IEquatable<ImmutableAppearance>{
public sealed class ImmutableAppearance : IEquatable<ImmutableAppearance> {
private uint? _registeredId;
private bool _needsFinalizer;
private int? _storedHashCode;
Expand Down Expand Up @@ -255,7 +255,7 @@ public override int GetHashCode() {
return (int)_storedHashCode;
}

public ImmutableAppearance(NetIncomingMessage buffer, IRobustSerializer serializer) {
public ImmutableAppearance(NetIncomingMessage buffer, IRobustSerializer serializer) {
Overlays = [];
Underlays = [];
VisContents = [];
Expand Down Expand Up @@ -418,44 +418,41 @@ public ImmutableAppearance(NetIncomingMessage buffer, IRobustSerializer serializ
//Creates an editable *copy* of this appearance, which must be added to the ServerAppearanceSystem to be used.
[Pure]
public MutableAppearance ToMutable() {
MutableAppearance result = new MutableAppearance() {
Name = Name,
Icon = Icon,
IconState = IconState,
Direction = Direction,
InheritsDirection = InheritsDirection,
PixelOffset = PixelOffset,
PixelOffset2 = PixelOffset2,
Color = Color,
Alpha = Alpha,
GlideSize = GlideSize,
ColorMatrix = ColorMatrix,
Layer = Layer,
Plane = Plane,
RenderSource = RenderSource,
RenderTarget = RenderTarget,
BlendMode = BlendMode,
AppearanceFlags = AppearanceFlags,
Invisibility = Invisibility,
Opacity = Opacity,
MouseOpacity = MouseOpacity,
Overlays = new(Overlays.Length),
Underlays = new(Underlays.Length),
VisContents = new(VisContents),
Filters = new(Filters),
Verbs = new(Verbs),
Override = Override,
};

foreach(ImmutableAppearance overlay in Overlays)
result.Overlays.Add(overlay);

foreach(ImmutableAppearance underlay in Underlays)
result.Underlays.Add(underlay);

for (int i = 0; i < 6; i++) {
result.Transform[i] = Transform[i];
}
MutableAppearance result = MutableAppearance.Get();

result.Name = Name;
result.Icon = Icon;
result.IconState = IconState;
result.Direction = Direction;
result.InheritsDirection = InheritsDirection;
result.PixelOffset = PixelOffset;
result.PixelOffset2 = PixelOffset2;
result.Color = Color;
result.Alpha = Alpha;
result.GlideSize = GlideSize;
result.ColorMatrix = ColorMatrix;
result.Layer = Layer;
result.Plane = Plane;
result.RenderSource = RenderSource;
result.RenderTarget = RenderTarget;
result.BlendMode = BlendMode;
result.AppearanceFlags = AppearanceFlags;
result.Invisibility = Invisibility;
result.Opacity = Opacity;
result.MouseOpacity = MouseOpacity;
result.Override = Override;

result.Overlays.EnsureCapacity(Overlays.Length);
result.Underlays.EnsureCapacity(Underlays.Length);
result.VisContents.EnsureCapacity(VisContents.Length);
result.Filters.EnsureCapacity(Filters.Length);
result.Verbs.EnsureCapacity(Verbs.Length);
result.Overlays.AddRange(Overlays);
result.Underlays.AddRange(Underlays);
result.VisContents.AddRange(VisContents);
result.Filters.AddRange(Filters);
result.Verbs.AddRange(Verbs);
Array.Copy(Transform, result.Transform, 6);

return result;
}
Expand Down
56 changes: 40 additions & 16 deletions OpenDreamShared/Dream/MutableAppearance.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ namespace OpenDreamShared.Dream;

// TODO: Wow this is huge! Probably look into splitting this by most used/least used to reduce the size of these
[Serializable]
public sealed class MutableAppearance : IEquatable<MutableAppearance>{
public sealed class MutableAppearance : IEquatable<MutableAppearance>, IDisposable {
public static readonly MutableAppearance Default = new();

private static Stack<MutableAppearance> _mutableAppearancePool = new();

[ViewVariables] public string Name = string.Empty;
[ViewVariables] public int? Icon;
[ViewVariables] public string? IconState;
Expand Down Expand Up @@ -72,15 +74,34 @@ public sealed class MutableAppearance : IEquatable<MutableAppearance>{
// PixelOffset2 behaves the same as PixelOffset in top-down mode, so this is used
public Vector2i TotalPixelOffset => PixelOffset + PixelOffset2;

public MutableAppearance() {
Overlays = new();
Underlays = new();
VisContents = new();
Filters = new();
Verbs = new();
private MutableAppearance() {
Overlays = [];
Underlays = [];
VisContents = [];
Filters = [];
Verbs = [];
}

public void Dispose() {
CopyFrom(Default);
_mutableAppearancePool.Push(this);
}

public static MutableAppearance Get() {
if (_mutableAppearancePool.TryPop(out var popped))
return popped;

return new MutableAppearance();
}

public MutableAppearance(MutableAppearance appearance) {
public static MutableAppearance GetCopy(MutableAppearance appearance) {
MutableAppearance result = Get();

result.CopyFrom(appearance);
return result;
}

public void CopyFrom(MutableAppearance appearance) {
Name = appearance.Name;
Icon = appearance.Icon;
IconState = appearance.IconState;
Expand All @@ -101,16 +122,19 @@ public MutableAppearance(MutableAppearance appearance) {
Invisibility = appearance.Invisibility;
Opacity = appearance.Opacity;
MouseOpacity = appearance.MouseOpacity;
Overlays = new(appearance.Overlays);
Underlays = new(appearance.Underlays);
VisContents = new(appearance.VisContents);
Filters = new(appearance.Filters);
Verbs = new(appearance.Verbs);
Override = appearance.Override;

for (int i = 0; i < 6; i++) {
Transform[i] = appearance.Transform[i];
}
Overlays.Clear();
Underlays.Clear();
VisContents.Clear();
Filters.Clear();
Verbs.Clear();
Overlays.AddRange(appearance.Overlays);
Underlays.AddRange(appearance.Underlays);
VisContents.AddRange(appearance.VisContents);
Filters.AddRange(appearance.Filters);
Verbs.AddRange(appearance.Verbs);
Array.Copy(appearance.Transform, Transform, 6);
}

public override bool Equals(object? obj) => obj is MutableAppearance appearance && Equals(appearance);
Expand Down

0 comments on commit 4b548c5

Please sign in to comment.