Skip to content

Commit

Permalink
Remove the Base cache because it's not practically useful (#183)
Browse files Browse the repository at this point in the history
  • Loading branch information
adamhathcock authored Dec 3, 2024
1 parent 1844386 commit daec64c
Show file tree
Hide file tree
Showing 7 changed files with 23 additions and 65 deletions.
31 changes: 7 additions & 24 deletions src/Speckle.Sdk/Serialisation/V2/Send/ObjectSerializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ public class ObjectSerializer : IObjectSerializer
{
private HashSet<object> _parentObjects = new();
private readonly Dictionary<Id, int> _currentClosures = new();
private readonly IDictionary<Base, CacheInfo> _baseCache;

private readonly bool _trackDetachedChildren;
private readonly IBasePropertyGatherer _propertyGatherer;
Expand All @@ -41,12 +40,10 @@ public class ObjectSerializer : IObjectSerializer
/// <param name="cancellationToken"></param>
public ObjectSerializer(
IBasePropertyGatherer propertyGatherer,
IDictionary<Base, CacheInfo> baseCache,
bool trackDetachedChildren = false,
CancellationToken cancellationToken = default
)
{
_baseCache = baseCache;
_propertyGatherer = propertyGatherer;
_cancellationToken = cancellationToken;
_trackDetachedChildren = trackDetachedChildren;
Expand All @@ -69,7 +66,6 @@ public ObjectSerializer(
{
throw new SpeckleSerializeException($"Failed to extract (pre-serialize) properties from the {baseObj}", ex);
}
_baseCache[baseObj] = new(item.Item2, _currentClosures);
yield return (item.Item1, item.Item2);
foreach (var chunk in _chunks)
{
Expand Down Expand Up @@ -232,26 +228,13 @@ private void SerializeProperty(object? obj, JsonWriter writer, PropertyAttribute
return null;
}

Closures childClosures;
Id id;
Json json;
if (_baseCache.TryGetValue(baseObj, out var info))
{
id = new Id(baseObj.id);
childClosures = info.Closures;
json = info.Json;
MergeClosures(_currentClosures, childClosures);
}
else
{
childClosures = isRoot || inheritedDetachInfo.IsDetachable ? _currentClosures : [];
var sb = Pools.StringBuilders.Get();
using var writer = new StringWriter(sb);
using var jsonWriter = SpeckleObjectSerializerPool.Instance.GetJsonTextWriter(writer);
id = SerializeBaseObject(baseObj, jsonWriter, childClosures);
json = new Json(writer.ToString());
Pools.StringBuilders.Return(sb);
}
var childClosures = isRoot || inheritedDetachInfo.IsDetachable ? _currentClosures : [];
var sb = Pools.StringBuilders.Get();
using var writer = new StringWriter(sb);
using var jsonWriter = SpeckleObjectSerializerPool.Instance.GetJsonTextWriter(writer);
var id = SerializeBaseObject(baseObj, jsonWriter, childClosures);
var json = new Json(writer.ToString());
Pools.StringBuilders.Return(sb);

_parentObjects.Remove(baseObj);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
using Speckle.InterfaceGenerator;
using Speckle.Sdk.Models;

namespace Speckle.Sdk.Serialisation.V2.Send;

[GenerateAutoInterface]
public class ObjectSerializerFactory(IBasePropertyGatherer propertyGatherer) : IObjectSerializerFactory
{
public IObjectSerializer Create(IDictionary<Base, CacheInfo> baseCache, CancellationToken cancellationToken) =>
new ObjectSerializer(propertyGatherer, baseCache, true, cancellationToken);
public IObjectSerializer Create(CancellationToken cancellationToken) =>
new ObjectSerializer(propertyGatherer, true, cancellationToken);
}
9 changes: 3 additions & 6 deletions src/Speckle.Sdk/Serialisation/V2/Send/SerializeProcess.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

namespace Speckle.Sdk.Serialisation.V2.Send;

public record SerializeProcessOptions(bool SkipCacheRead, bool SkipCacheWrite, bool SkipServer, bool CacheBases);
public record SerializeProcessOptions(bool SkipCacheRead, bool SkipCacheWrite, bool SkipServer);

public readonly record struct SerializeProcessResults(
string RootId,
Expand All @@ -26,11 +26,8 @@ public class SerializeProcess(
SerializeProcessOptions? options = null
) : ChannelSaver, ISerializeProcess
{
private readonly SerializeProcessOptions _options = options ?? new(false, false, false, true);
private readonly SerializeProcessOptions _options = options ?? new(false, false, false);
private readonly ConcurrentDictionary<Id, Json> _jsonCache = new();

private readonly IDictionary<Base, CacheInfo> _baseCache =
options?.CacheBases ?? true ? new ConcurrentDictionary<Base, CacheInfo>() : new EmptyDictionary<Base, CacheInfo>();
private readonly ConcurrentDictionary<Id, ObjectReference> _objectReferences = new();

private long _totalFound;
Expand Down Expand Up @@ -110,7 +107,7 @@ private IEnumerable<BaseItem> Serialise(Base obj, CancellationToken cancellation
}
if (id is null || !_jsonCache.TryGetValue(id.Value, out var json))
{
var serializer2 = objectSerializerFactory.Create(_baseCache, cancellationToken);
var serializer2 = objectSerializerFactory.Create(cancellationToken);
var items = serializer2.Serialize(obj).ToList();
foreach (var kvp in serializer2.ObjectReferences)
{
Expand Down
2 changes: 1 addition & 1 deletion tests/Speckle.Sdk.Serialization.Testing/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
streamId,
token,
progress,
new SerializeProcessOptions(skipCacheSendCheck, skipCacheSendSave, true, true)
new SerializeProcessOptions(skipCacheSendCheck, skipCacheSendSave, true)
);
await process2.Serialize(@base, default).ConfigureAwait(false);
Console.WriteLine("Detach");
Expand Down
12 changes: 4 additions & 8 deletions tests/Speckle.Sdk.Serialization.Tests/DetachedTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@ public void Setup()
}

[Test(Description = "Checks that all typed properties (including obsolete ones) are returned")]
[TestCase(true)]
[TestCase(false)]
public async Task CanSerialize_New_Detached(bool cacheBases)
public async Task CanSerialize_New_Detached()
{
var expectedJson = """
{
Expand Down Expand Up @@ -76,7 +74,7 @@ public async Task CanSerialize_New_Detached(bool cacheBases)
new DummyServerObjectManager(),
new BaseChildFinder(new BasePropertyGatherer()),
new ObjectSerializerFactory(new BasePropertyGatherer()),
new SerializeProcessOptions(false, false, true, cacheBases)
new SerializeProcessOptions(false, false, true)
);
await process2.Serialize(@base, default).ConfigureAwait(false);

Expand Down Expand Up @@ -193,9 +191,7 @@ public void GetPropertiesExpected_All()
}

[Test(Description = "Checks that all typed properties (including obsolete ones) are returned")]
[TestCase(true)]
[TestCase(false)]
public async Task CanSerialize_New_Detached2(bool cacheBases)
public async Task CanSerialize_New_Detached2()
{
var root = """
Expand Down Expand Up @@ -270,7 +266,7 @@ public async Task CanSerialize_New_Detached2(bool cacheBases)
new DummyServerObjectManager(),
new BaseChildFinder(new BasePropertyGatherer()),
new ObjectSerializerFactory(new BasePropertyGatherer()),
new SerializeProcessOptions(false, false, true, cacheBases)
new SerializeProcessOptions(false, false, true)
);
var results = await process2.Serialize(@base, default).ConfigureAwait(false);

Expand Down
27 changes: 5 additions & 22 deletions tests/Speckle.Sdk.Serialization.Tests/ExternalIdTests.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System.Collections.Concurrent;
using NUnit.Framework;
using NUnit.Framework;
using Shouldly;
using Speckle.Newtonsoft.Json.Linq;
using Speckle.Objects.Geometry;
Expand Down Expand Up @@ -27,11 +26,7 @@ public void Setup()
public void ExternalIdTest_Detached(string lineId, string valueId)
{
var p = new Polyline() { units = "cm", value = [1, 2] };
var serializer = new ObjectSerializer(
new BasePropertyGatherer(),
new ConcurrentDictionary<Base, CacheInfo>(),
true
);
var serializer = new ObjectSerializer(new BasePropertyGatherer(), true);
var list = serializer.Serialize(p).ToDictionary(x => x.Item1, x => x.Item2);
list.ContainsKey(new Id(lineId)).ShouldBeTrue();
var json = list[new Id(lineId)];
Expand All @@ -58,11 +53,7 @@ public void ExternalIdTest_Detached_Nested(string lineId, string valueId)
knots = [],
weights = [],
};
var serializer = new ObjectSerializer(
new BasePropertyGatherer(),
new ConcurrentDictionary<Base, CacheInfo>(),
true
);
var serializer = new ObjectSerializer(new BasePropertyGatherer(), true);
var list = serializer.Serialize(curve).ToDictionary(x => x.Item1, x => x.Item2);
list.ContainsKey(new Id(lineId)).ShouldBeTrue();
var json = list[new Id(lineId)];
Expand Down Expand Up @@ -90,11 +81,7 @@ public void ExternalIdTest_Detached_Nested_More(string lineId, string valueId)
weights = [],
};
var polycurve = new Polycurve() { segments = [curve], units = "cm" };
var serializer = new ObjectSerializer(
new BasePropertyGatherer(),
new ConcurrentDictionary<Base, CacheInfo>(),
true
);
var serializer = new ObjectSerializer(new BasePropertyGatherer(), true);
var list = serializer.Serialize(polycurve).ToDictionary(x => x.Item1, x => x.Item2);
list.ContainsKey(new Id(lineId)).ShouldBeTrue();
var json = list[new Id(lineId)];
Expand Down Expand Up @@ -124,11 +111,7 @@ public void ExternalIdTest_Detached_Nested_More_Too(string lineId, string valueI
var polycurve = new Polycurve() { segments = [curve], units = "cm" };
var @base = new Base();
@base.SetDetachedProp("profile", polycurve);
var serializer = new ObjectSerializer(
new BasePropertyGatherer(),
new ConcurrentDictionary<Base, CacheInfo>(),
true
);
var serializer = new ObjectSerializer(new BasePropertyGatherer(), true);
var list = serializer.Serialize(@base).ToDictionary(x => x.Item1, x => x.Item2);
list.ContainsKey(new Id(lineId)).ShouldBeTrue();
var json = list[new Id(lineId)];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ public async Task Roundtrip_Test_New(string fileName, string rootId, int oldCoun
new DummySendServerObjectManager(newIdToJson),
new BaseChildFinder(new BasePropertyGatherer()),
new ObjectSerializerFactory(new BasePropertyGatherer()),
new SerializeProcessOptions(true, true, false, true)
new SerializeProcessOptions(true, true, false)
);
var (rootId2, _) = await serializeProcess.Serialize(root, default);

Expand Down

0 comments on commit daec64c

Please sign in to comment.