-
Notifications
You must be signed in to change notification settings - Fork 1k
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.
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
Fix persistence allocations part 2. #6487
Fix persistence allocations part 2. #6487
Conversation
public bool Equals(WriteMessages other) | ||
{ | ||
if (ReferenceEquals(other, null)) return false; | ||
if (ReferenceEquals(this, other)) return true; | ||
|
||
return Equals(ActorInstanceId, other.ActorInstanceId) | ||
return ActorInstanceId == other.ActorInstanceId |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rider highligths such cases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have a question about changing the order of the stashing invocations on the persistent actors, but otherwise it looks good.
{ | ||
_journalBatch.Add(p); | ||
} | ||
_eventBatch = new LinkedList<IPersistentEnvelope>(); | ||
_eventBatch.Clear(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch.
@@ -341,7 +341,7 @@ private void HandleWriteMessages(WriteMessages message) | |||
*/ | |||
var self = Self; | |||
_resequencerCounter += message.Messages.Aggregate(1, (acc, m) => acc + m.Size); | |||
var atomicWriteCount = message.Messages.OfType<AtomicWrite>().Count(); | |||
var atomicWriteCount = message.Messages.Count(x => x is AtomicWrite); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Changes
Next optimization of persistence allocation that was started in #5505
Now excessive object allocation was eliminated in Eventsourced class when using linked list. Benchmark shows kinda 10% lower memory usage.
Checklist
Latest
dev
BenchmarksBenchmarkDotNet=v0.13.2, OS=macOS Monterey 12.5.1 (21G83) [Darwin 21.6.0]
Apple M1 2.40GHz, 1 CPU, 8 logical and 8 physical cores
.NET SDK=7.0.101
[Host] : .NET 7.0.1 (7.0.122.56804), X64 RyuJIT SSE4.2 DEBUG
Toolchain=InProcessEmitToolchain InvocationCount=1 UnrollFactor=1
This PR's Benchmarks
BenchmarkDotNet=v0.13.2, OS=macOS Monterey 12.5.1 (21G83) [Darwin 21.6.0]
Apple M1 2.40GHz, 1 CPU, 8 logical and 8 physical cores
.NET SDK=7.0.101
[Host] : .NET 7.0.1 (7.0.122.56804), X64 RyuJIT SSE4.2 DEBUG
Toolchain=InProcessEmitToolchain InvocationCount=1 UnrollFactor=1