Provides generic version of LifetimeEntryManager
#6212
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,24 +1,30 @@ | ||
| // Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence. | ||
| // See the LICENCE file in the repository root for full licence text. | ||
|
|
||
| using System; | ||
|
|
||
| namespace osu.Framework.Graphics.Performance | ||
| { | ||
| /// <summary> | ||
| /// An object for a <see cref="LifetimeEntryManager"/> to consume, which provides a <see cref="LifetimeEntry{T}.LifetimeStart"/> and <see cref="LifetimeEntry{T}.LifetimeEnd"/>. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// Management of the object which the <see cref="LifetimeEntry"/> refers to is left up to the consumer. | ||
| /// </remarks> | ||
| public class LifetimeEntry : LifetimeEntry<LifetimeEntry> | ||
| { | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// The required base type for the <see cref="LifetimeEntryManager{T}"/> to consume, which provides a <see cref="LifetimeEntry{T}.LifetimeStart"/> and <see cref="LifetimeEntry{T}.LifetimeEnd"/>. | ||
| /// </summary> | ||
| /// <typeparam name="TDerived">The implemented class itself. Used to provide derived category information for base categories using the Curiously Recurring Template Pattern(CRTP).</typeparam> | ||
| public abstract class LifetimeEntry<TDerived> where TDerived : LifetimeEntry<TDerived> | ||
| { | ||
| private double lifetimeStart = double.MinValue; | ||
|
|
||
| /// <summary> | ||
| /// The time at which this <see cref="LifetimeEntry{T}"/> becomes alive in a <see cref="LifetimeEntryManager{T}"/>. | ||
| /// </summary> | ||
| public double LifetimeStart | ||
| { | ||
|
|
@@ -30,7 +36,7 @@ public double LifetimeStart | |
| private double lifetimeEnd = double.MaxValue; | ||
|
|
||
| /// <summary> | ||
| /// The time at which this <see cref="LifetimeEntry{T}"/> becomes dead in a <see cref="LifetimeEntryManager{T}"/>. | ||
| /// </summary> | ||
| public double LifetimeEnd | ||
| { | ||
|
|
@@ -42,15 +48,15 @@ public double LifetimeEnd | |
| /// Invoked before <see cref="LifetimeStart"/> or <see cref="LifetimeEnd"/> changes. | ||
| /// It is used because <see cref="LifetimeChanged"/> cannot be used to ensure comparator stability. | ||
| /// </summary> | ||
| internal event Action<TDerived>? RequestLifetimeUpdate; | ||
|
|
||
| /// <summary> | ||
| /// Invoked after <see cref="LifetimeStart"/> or <see cref="LifetimeEnd"/> changes. | ||
| /// </summary> | ||
| public event Action<TDerived>? LifetimeChanged; | ||
|
|
||
| /// <summary> | ||
| /// Update <see cref="LifetimeStart"/> of this <see cref="LifetimeEntry{T}"/>. | ||
| /// </summary> | ||
| protected virtual void SetLifetimeStart(double start) | ||
| { | ||
|
|
@@ -59,7 +65,7 @@ protected virtual void SetLifetimeStart(double start) | |
| } | ||
|
|
||
| /// <summary> | ||
| /// Update <see cref="LifetimeEnd"/> of this <see cref="LifetimeEntry{T}"/>. | ||
| /// </summary> | ||
| protected virtual void SetLifetimeEnd(double end) | ||
| { | ||
|
|
@@ -68,27 +74,44 @@ protected virtual void SetLifetimeEnd(double end) | |
| } | ||
|
|
||
| /// <summary> | ||
| /// Updates the stored lifetimes of this <see cref="LifetimeEntry{T}"/>. | ||
| /// </summary> | ||
| /// <param name="start">The new <see cref="LifetimeStart"/> value.</param> | ||
| /// <param name="end">The new <see cref="LifetimeEnd"/> value.</param> | ||
| protected void SetLifetime(double start, double end) | ||
| { | ||
| // Due to the type constraints of C#, we cannot declare `LifetimeEntry<T> where T = LifetimeEntry<T>` to limit the type of `this` to be `T`. But when used correctly, this will always be the case. | ||
| // aka. We can't stop anyone from writing code like this: | ||
| // ```csharp | ||
| // public class MyEntry : LifetimeEntry<MyEntry> { } | ||
| // LifetimeEntry<MyEntry> foo = new(); | ||
| // ``` | ||
| // We prevent users from inadvertently writing such code by declaring `LifetimeEntry<T>` as `abstract`, but we cannot prevent it completely. | ||
| // ```csharp | ||
| // public class NewEntry<T> : LifetimeEntry<T> where T : LifetimeEntry<T> { } | ||
| // NewEntry<MyEntry> a = new(); | ||
| // ``` | ||
| // Happily, however, the compiler correctly prevents code like this from compiling. This is also enough to deter users from writing incorrect code: | ||
| // ```csharp | ||
| // public class ErrorEntry : NewEntry<MyEntry> { } | ||
| // LifetimeEntryManager<NewEntry<MyEntry>> error1 = new(); // Compiler error. | ||
| // LifetimeEntryManager<ErrorEntry> error2 = new(); // Compiler error. | ||
| // ``` | ||
| RequestLifetimeUpdate?.Invoke((TDerived)this); | ||
|
Comment on lines
+83
to
+100
There was a problem hiding this comment. Case in point for the above diatribe. If API correctness of a method requires this sort of preamble to convince readers that the API surface is safe, the whole thing is overdone. There was a problem hiding this comment. Basically, it's the same thing, I'm just trying to illustrate why we still can't get away from typecasting. I simply think that a more detailed explanation will help myself or other users understand the design logic clearly. These comments are not actually necessary, since I don't really need any additional constraints or commitments. These are written within the design scope of the compiler and I'm not trying to break anything. I tried to clarify my thinking through comments, but now it seems like this is overkill? There was a problem hiding this comment. I'm not even sure what that is attempting to explain. Why is the first snippet "unwanted"? It looks like perfectly reasonable code. What is the second snippet supposed to be? What are There was a problem hiding this comment.
Because if the user writes code like that, the cast here will throw an exception. LifetimeEntry<MyEntry>
// TDerived is "MyEntry" here.
// this.GetType() == LifetimeEntry
...
(MyEntry)this // throw an Exception: A base type cannot be cast to a derived type.Also notice that I declared this class as an abstract class to prevent users from doing something wrong like this. There was a problem hiding this comment. However, this subtle error will be exposed through the following compilation errors when writing. I just want to make this clear. After all, this recursive form of generics can be a bit confusing to understand how the compiler will handle it. There was a problem hiding this comment.
Assumptions about an extreme situation. Although I declared this class as abstract class. But like all abstract classes there are problems that may be encountered. You can't prevent users from implementing an abstract class without taking any meaningful action. This is basically like: abstract class A {
// some thing here.
}
class B : A { } // class B is the same as A, but it's no longer abstract!This makes it possible for users to write illogical code. Then in the third code block, I actually illustrate that even if this happens, the compiler will correctly prevent the user from doing this. |
||
|
|
||
| lifetimeStart = start; | ||
| lifetimeEnd = Math.Max(start, end); // Negative intervals are undesired. | ||
|
|
||
| LifetimeChanged?.Invoke((TDerived)this); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// The current state of this <see cref="LifetimeEntry{T}"/>. | ||
| /// </summary> | ||
| internal LifetimeEntryState State { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Uniquely identifies this <see cref="LifetimeEntry{T}"/> in a <see cref="LifetimeEntryManager{T}"/>. | ||
| /// </summary> | ||
| internal ulong ChildId { get; set; } | ||
| } | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,12 @@ | ||
| // Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence. | ||
| // See the LICENCE file in the repository root for full licence text. | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Diagnostics; | ||
|
|
||
| using JetBrains.Annotations; | ||
|
|
||
| using osu.Framework.Extensions.ObjectExtensions; | ||
| using osu.Framework.Graphics.Containers; | ||
| using osu.Framework.Statistics; | ||
|
|
@@ -16,49 +16,58 @@ namespace osu.Framework.Graphics.Performance | |
| /// <summary> | ||
| /// Provides time-optimised updates for lifetime change notifications. | ||
| /// This is used in specialised <see cref="CompositeDrawable"/>s to optimise lifetime changes (see: <see cref="LifetimeManagementContainer"/>). | ||
| /// If you wish to implement your own <see cref="LifetimeEntry{T}"/>, please use the generic version <see cref="LifetimeEntryManager{T}"/>. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// The time complexity of updating lifetimes is O(number of alive items). | ||
| /// </remarks> | ||
| public class LifetimeEntryManager : LifetimeEntryManager<LifetimeEntry> | ||
| { | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Provides time-optimised updates for lifetime change notifications. | ||
| /// </summary> | ||
| /// <typeparam name="T">Implementation of <see cref="LifetimeEntry{T}"/>. Note that your own class must inherit from the generic version <see cref="LifetimeEntry{T}"/> rather than <see cref="LifetimeEntry"/>.</typeparam> | ||
| public class LifetimeEntryManager<T> where T : LifetimeEntry<T> | ||
| { | ||
| /// <summary> | ||
| /// Invoked immediately when a <see cref="LifetimeEntry{T}"/> becomes alive. | ||
| /// </summary> | ||
| public event Action<T>? EntryBecameAlive; | ||
|
|
||
| /// <summary> | ||
| /// Invoked immediately when a <see cref="LifetimeEntry{T}"/> becomes dead. | ||
| /// </summary> | ||
| public event Action<T>? EntryBecameDead; | ||
|
|
||
| /// <summary> | ||
| /// Invoked when a <see cref="LifetimeEntry{T}"/> crosses a lifetime boundary. | ||
| /// </summary> | ||
| public event Action<T, LifetimeBoundaryKind, LifetimeBoundaryCrossingDirection>? EntryCrossedBoundary; | ||
|
|
||
| /// <summary> | ||
| /// Contains all the newly-added (but not yet processed) entries. | ||
| /// </summary> | ||
| private readonly List<T> newEntries = []; | ||
|
|
||
| /// <summary> | ||
| /// Contains all the currently-alive entries. | ||
| /// </summary> | ||
| private readonly List<T> activeEntries = []; | ||
|
|
||
| /// <summary> | ||
| /// Contains all entries that should come alive in the future. | ||
| /// </summary> | ||
| private readonly SortedSet<T> futureEntries = new(new LifetimeStartComparator()); | ||
|
|
||
| /// <summary> | ||
| /// Contains all entries that were alive in the past. | ||
| /// </summary> | ||
| private readonly SortedSet<T> pastEntries = new(new LifetimeEndComparator()); | ||
|
|
||
| private readonly Queue<(T, LifetimeBoundaryKind, LifetimeBoundaryCrossingDirection)> eventQueue = | ||
| new(); | ||
|
|
||
| /// <summary> | ||
| /// Used to ensure a stable sort if multiple entries with the same lifetime are added. | ||
|
|
@@ -68,8 +77,8 @@ public class LifetimeEntryManager | |
| /// <summary> | ||
| /// Adds an entry. | ||
| /// </summary> | ||
| /// <param name="entry">The <see cref="LifetimeEntry{T}"/> to add.</param> | ||
| public void AddEntry(T entry) | ||
| { | ||
| entry.RequestLifetimeUpdate += requestLifetimeUpdate; | ||
| entry.ChildId = ++currentChildId; | ||
|
|
@@ -81,9 +90,9 @@ public void AddEntry(LifetimeEntry entry) | |
| /// <summary> | ||
| /// Removes an entry. | ||
| /// </summary> | ||
| /// <param name="entry">The <see cref="LifetimeEntry{T}"/> to remove.</param> | ||
| /// <returns>Whether <paramref name="entry"/> was contained by this <see cref="LifetimeEntryManager{T}"/>.</returns> | ||
| public bool RemoveEntry(T entry) | ||
| { | ||
| entry.RequestLifetimeUpdate -= requestLifetimeUpdate; | ||
|
|
||
|
|
@@ -160,7 +169,7 @@ public void ClearEntries() | |
| /// <summary> | ||
| /// Invoked when the lifetime of an entry is going to changed. | ||
| /// </summary> | ||
| private void requestLifetimeUpdate(T entry) | ||
| { | ||
| // Entries in the past/future sets need to be re-sorted to prevent the comparer from becoming unstable. | ||
| // To prevent, e.g. CompositeDrawable alive children changing during enumeration, the entry's state must not be updated immediately. | ||
|
|
@@ -185,7 +194,7 @@ private void requestLifetimeUpdate(LifetimeEntry entry) | |
| /// <param name="state">The <see cref="LifetimeEntryState"/>.</param> | ||
| /// <returns>Either <see cref="futureEntries"/>, <see cref="pastEntries"/>, or null.</returns> | ||
| [CanBeNull] | ||
| private SortedSet<T>? futureOrPastEntries(LifetimeEntryState state) | ||
| { | ||
| switch (state) | ||
| { | ||
|
|
@@ -274,17 +283,17 @@ public bool Update(double startTime, double endTime) | |
| } | ||
|
|
||
| /// <summary> | ||
| /// Updates the state of a single <see cref="LifetimeEntry{T}"/>. | ||
| /// </summary> | ||
| /// <param name="entry">The <see cref="LifetimeEntry{T}"/> to update.</param> | ||
| /// <param name="startTime">The start of the time range.</param> | ||
| /// <param name="endTime">The end of the time range.</param> | ||
| /// <param name="isNewEntry">Whether <paramref name="entry"/> is part of the new entries set. | ||
| /// The state may be "new" or "past"/"future", in which case it will undergo further processing to return it to the correct set.</param> | ||
| /// <param name="mutateActiveEntries">Whether <see cref="activeEntries"/> should be mutated by this invocation. | ||
| /// If <c>false</c>, the caller is expected to handle mutation of <see cref="activeEntries"/> based on any changes to the entry's state.</param> | ||
| /// <returns>Whether the state of <paramref name="entry"/> has changed.</returns> | ||
| private bool updateChildEntry(T entry, double startTime, double endTime, bool isNewEntry, bool mutateActiveEntries) | ||
| { | ||
| LifetimeEntryState oldState = entry.State; | ||
|
|
||
|
|
@@ -343,11 +352,11 @@ private bool updateChildEntry(LifetimeEntry entry, double startTime, double endT | |
| /// <summary> | ||
| /// Retrieves the new state for an entry. | ||
| /// </summary> | ||
| /// <param name="entry">The <see cref="LifetimeEntry{T}"/>.</param> | ||
| /// <param name="startTime">The start of the time range.</param> | ||
| /// <param name="endTime">The end of the time range.</param> | ||
| /// <returns>The state of <paramref name="entry"/>. Can be either <see cref="LifetimeEntryState.Past"/>, <see cref="LifetimeEntryState.Current"/>, or <see cref="LifetimeEntryState.Future"/>.</returns> | ||
| private LifetimeEntryState getState(T entry, double startTime, double endTime) | ||
| { | ||
| // Consider a static entry and a moving time range: | ||
| // [-----------Entry-----------] | ||
|
|
@@ -367,7 +376,7 @@ private LifetimeEntryState getState(LifetimeEntry entry, double startTime, doubl | |
| return LifetimeEntryState.Current; | ||
| } | ||
|
|
||
| private void enqueueEvents(T entry, LifetimeEntryState oldState, LifetimeEntryState newState) | ||
| { | ||
| Debug.Assert(oldState != newState); | ||
|
|
||
|
|
@@ -394,11 +403,11 @@ private void enqueueEvents(LifetimeEntry entry, LifetimeEntryState oldState, Lif | |
| } | ||
|
|
||
| /// <summary> | ||
| /// Compares by <see cref="LifetimeEntry{T}.LifetimeStart"/>. | ||
| /// </summary> | ||
| private sealed class LifetimeStartComparator : IComparer<T> | ||
| { | ||
| public int Compare(T? x, T? y) | ||
| { | ||
| ArgumentNullException.ThrowIfNull(x); | ||
| ArgumentNullException.ThrowIfNull(y); | ||
|
|
@@ -409,11 +418,11 @@ public int Compare(LifetimeEntry x, LifetimeEntry y) | |
| } | ||
|
|
||
| /// <summary> | ||
| /// Compares by <see cref="LifetimeEntry{T}.LifetimeEnd"/>. | ||
| /// </summary> | ||
| private sealed class LifetimeEndComparator : IComparer<T> | ||
| { | ||
| public int Compare(T? x, T? y) | ||
| { | ||
| ArgumentNullException.ThrowIfNull(x); | ||
| ArgumentNullException.ThrowIfNull(y); | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Nooooooooooooooooooooooooope. Nope nope nope nope. I'm not having anything that the cpp people invented. Especially not with a name like "curiously recurring". I don't even follow what this is supposed to be doing.
I will not accept this sort of arcane construction here. If this makes-or-breaks the entire series then it might as well be closed right now.
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.
Sadly, this is necessary. Manager needs to use this type argument to get the actual type of the field.
I even thought this was a fairly common way of writing modern C#. I just want to clarify here what the real name of this mode is.😕
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.
I'm honestly having a difficult time accepting that linked article if it's giving
IEquatable<T>as an example of this.IEquatable<T>is nowhere near as difficult to parse! It's not doing thisIEquatable<T> where T : IEquatable<T>brain twister thing in any class signature. Its methods acceptT.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.
I just spent some time searching for something I myself learned about this many years ago. Luckily, I found it and I thought this article might be a good explanation.