Add new workspace event that gives handlers the opportunity to be processed immediately #76932

ToddGrun · 2025-01-26T16:13:35Z

This is in response to allocations seen when the CA process attempts to operate on a sourcetext version that has not been synchronized over. In those cases, the OOP process requests a full deserialization of the source text causing full buffer allocations in both VS and the CA process.

There are several points of asynchronicity in the current system around sending over buffer changes, reducing any of which would reduce the likelihood of needing this full serialization.

This PR removes one of those points of asynchronicity, specifically in the workspace eventing layer. Previously, all notiications were done on a delayed basis, by wrapping each notification in the WorkspaceSpace.ScheduleTask call from the RaiseWorkspaceChangedEventAsync method. This method allows callers to indicate they need to be called immediately upon the workspace change. As noted by a doc comment in the PR, these handlers should be very fast.

The numbers from speedometer came back looking pretty good (below). Another point of asynchronicity could be investigated around the usage of the _textChangeQueue ABWQ, but it might not be worth it as it looks like this change removes the majority of the serialization during typing. Other alternatives within the text syncing system such as allowing OOP to request a text change instead of the full buffer contents are also possible, but previous investigations into that ended up complicated and incomplete.

*** before allocations during typing in VS process ***

*** commit 3 allocations during typing in VS process ***

*** commit 17 allocations during typing in VS process ***

*** before allocations during typing in CA process ***

*** commit 3 allocations during typing in CA process ***

*** commit 17 allocations during typing in CA process ***

…e processed immediately This is in response to allocations seen when the OOP process attempts to operate on a sourcetext version that has not been synchronized over. In those cases, the OOP process requests a full deserialization of the source text causing full buffer allocations in both VS and the CA process. There are several points of asynchronicity in the current system around sending over buffer changes, reducing any of which would reduce the likelihood of needing this full serialization. This PR removes one of those points of asynchronicity, specifically in the workspace eventing layer. Previously, all notiications were done on a delayed basis, by wrapping each notification in the WorkspaceSpace.ScheduleTask call from the RaiseWorkspaceChangedEventAsync method. This method allows callers to indicate they need to be called immediately upon the workspace change. As noted by a doc comment in the PR, these handlers should be very fast. Going to mark this as draft and get speedometer numbers off this to see if this helps enough, or if instead the other points of asynchronicity should be investigated (specifically, the usage of the _textChangeQueue ABWQ). There are other alternatives within the text syncing system such as allowing OOP to request a text change instead of the full buffer contents, but previous investigations into that ended up complicated and incomplete.

ToddGrun · 2025-01-26T16:15:12Z

PR validation insertion: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/605941

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/FunctionId.cs

ToddGrun · 2025-01-27T03:36:19Z

Moving out of draft mode as the numbers look pretty good. Still some serialization happening during typing, but it's a lot less than before and this change was pretty trivial. If code reviewers think adding this to the workspace level is not desirable, can look into other approaches.

…ries

…r reduce the amount of serialization and deserialization

ToddGrun · 2025-01-27T14:42:11Z

commit 5 PR validation insertion: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/606025

…ositions to allow composition to include an IThreadingContext

…ecksumUpdater.DispatchSynchronizeTextChanges and RemoteAssetSynchronizationService.SynchronizeTextChangesAsync

src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs

...kspaces/Remote/ServiceHub/Services/AssetSynchronization/RemoteAssetSynchronizationService.cs

CyrusNajmabadi · 2025-01-28T01:31:25Z

src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs

-        using var _ = ArrayBuilder<(DocumentId id, Checksum textChecksum, ImmutableArray<TextChange> changes, Checksum newTextChecksum)>.GetInstance(out var builder);
-
-        foreach (var (oldDocument, newDocument) in values)
+        _ = _threadingContext.JoinableTaskFactory.RunAsync(async () =>


we should def doc the design here. make it clear why we've written it this way.

note: you can also use IAsyncToken here to track the work, allowing you to properly be able to unit test as well.

Is the intent here that this is fire-and-forget (and will still always yield the thread or not? Because there's absolutely a chance in VS this might be on the UI thread and we're calling those GetChangeRanges methods and such which might (?) be expensive. Or maybe not.

This indeed might be worth documenting, since it's otherwise unclear to me why we're wrapping this in a JTF.RunAsync().

The intent is to fire-and-forget, but only yield the current thread if an async operation necessitates it.

Is GetChangeRanges expensive? It doesn't look to be to me, but maybe I'm missing an override? If it is expensive, the two conditions that use it could probably be removed, as they are very minor and specialized optimizations.

Will add some doc here

I had thought GetChangeRanges could potentially end up in a text diff, but maybe not.

CyrusNajmabadi · 2025-01-28T01:32:51Z

src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs

+            using (Logger.LogBlock(FunctionId.Workspace_EventsImmediate, (s, p, d, k) => $"{s.Id} - {p} - {d} {kind.ToString()}", newSolution, projectId, documentId, kind, CancellationToken.None))
+            {
+                args = new WorkspaceChangeEventArgs(kind, oldSolution, newSolution, projectId, documentId);
+                ev.RaiseEvent(static (handler, arg) => handler(arg.self, arg.args), (self: this, args));


aside, are these exception safe if the attached handler throws an exception? i ask because the old system might have been ok ift he Task went into a faulted state. i'm not sure about the new system.

Oops asked this same question here: #76932 (comment) I think this might cause an issue now if this were to throw so probably best to handle that better.

Any concern with me just changing RaiseEvent from using FatalError.ReportAndPropagate to instead use FatalError.ReportAndCatch (also, would it make sense to have the try/catch inside the foreach in that method?)

@jasonmalinowski -- any thoughts on change RaiseEvent to instead use FatalError.ReportAndCatch (and to move the try/catch inside the loop)?

I went ahead and just made the change to switch to ReportAndCatch

src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs

jasonmalinowski · 2025-01-28T01:40:47Z

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/FunctionId.cs

    VSCode_Project_Load_Started = 861,
    VSCode_Projects_Load_Completed = 862,
+
+    // 900-999 for items that don't fit into other categories.


Suggested change

// 900-999 for items that don't fit into other categories.

// 900-999 things related to workspace and OOP solution sync

We're not restricted to three digits, so no reason to make this the final catch-all group.

I wanted a catch-all group as I was tired scanning through trying to find the next available slot with all the other groups. Is the request here to change the starting range / size of the catch all group, or just not to have it?

Just don't have it. And if we think groups don't make sense in the first place, just get rid of all the groups. 😄

i really like groups. just give things around 10-20 enries. it's def nicer when you're adding a bunch of new telemetry markers to a speciifc area.

src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs

jasonmalinowski · 2025-01-28T01:45:55Z

src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs

-        using var _ = ArrayBuilder<(DocumentId id, Checksum textChecksum, ImmutableArray<TextChange> changes, Checksum newTextChecksum)>.GetInstance(out var builder);
-
-        foreach (var (oldDocument, newDocument) in values)
+        _ = _threadingContext.JoinableTaskFactory.RunAsync(async () =>


Is the intent here that this is fire-and-forget (and will still always yield the thread or not? Because there's absolutely a chance in VS this might be on the UI thread and we're calling those GetChangeRanges methods and such which might (?) be expensive. Or maybe not.

This indeed might be worth documenting, since it's otherwise unclear to me why we're wrapping this in a JTF.RunAsync().

jasonmalinowski · 2025-01-28T01:48:40Z

src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs

+            using (Logger.LogBlock(FunctionId.Workspace_EventsImmediate, (s, p, d, k) => $"{s.Id} - {p} - {d} {kind.ToString()}", newSolution, projectId, documentId, kind, CancellationToken.None))
+            {
+                args = new WorkspaceChangeEventArgs(kind, oldSolution, newSolution, projectId, documentId);
+                ev.RaiseEvent(static (handler, arg) => handler(arg.self, arg.args), (self: this, args));


It appears RaiseEvent will report any exception being thrown but will still propagate it out. That might be dangerous here, because it now means any subscriber's mistake will now completely break the workspace and put the user in a really broken state. I admit I'm not sure why we aren't catching exceptions in RaiseEvent, so maybe that should be changed (or we have a variant that does catch).

use some local functions for cleanup Update some comments

CyrusNajmabadi

Signing mostly off. I do want Jason to approve as well. And there look to be a few places we can clean up. I would like exception resilience thought about. Thanks!

CyrusNajmabadi · 2025-01-28T04:05:39Z

src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs

        Workspace workspace,
        IAsynchronousOperationListenerProvider listenerProvider,
+        IThreadingContext threadingContext,
        CancellationToken shutdownToken)


I'm curious if the shutdown token is needed. Instead of just using the disposal token on the threading context

I think they are different. VisualStudioWorkspaceServiceHubConnector has StartListening/StopListening methods, which drive the shutdownToken. However, I think the threadingContext's DisposalToken is driven by MEF and I don't think is disposed until VS shutdown.

Just saw your other comment about VisualStudioWorkspaceServiceHubConnector. Let me think about that.

It looks like right now, VisualStudioWorkspaceServiceHubConnector.StopListening is called on Workspace dispose. If I just send over the threadingContext and use it's cancellation token, are we ok with essentially just using a cancellation token from MEF dispose and not workspace dispose?

yeah. let's keep this as is. i've looked at hte code and i'm not sure how to reconcile the different Disposes/CTs. So let's not change that now.

src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs

CyrusNajmabadi · 2025-01-28T04:08:49Z

src/VisualStudio/Core/Def/Remote/VisualStudioWorkspaceServiceHubConnector.cs


        // only push solution snapshot from primary (VS) workspace:
-        _checksumUpdater = new SolutionChecksumUpdater(workspace, _listenerProvider, _disposalCancellationSource.Token);
+        _checksumUpdater = new SolutionChecksumUpdater(workspace, _listenerProvider, _threadingContext, _disposalCancellationSource.Token);


If we have the threading context, we might be able to get rid of the disposal logic here and passing that ct along

src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs

...kspaces/Remote/ServiceHub/Services/AssetSynchronization/RemoteAssetSynchronizationService.cs

Update comments Use string constants instead of allocating

ToddGrun · 2025-01-31T18:08:28Z

@jasonmalinowski -- would love to get your feedback on the changes and any remaining concerns you may have that haven't been addressed as desired

src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs

CyrusNajmabadi · 2025-02-01T18:45:29Z

src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs

+            TelemetryLogging.LogAggregatedCounter(FunctionId.ChecksumUpdater_SynchronizeTextChangesStatus, KeyValueLogMessage.Create(m =>
+            {
+                m[TelemetryLogging.KeyName] = nameof(SolutionChecksumUpdater) + "." + metricName;
+                m[TelemetryLogging.KeyValue] = 1L;


is the idea that this will add 1 each time this happens?

Yes, the LogAggregatedCounter will add the specified amount to the counter

CyrusNajmabadi · 2025-02-01T18:45:51Z

src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs

+            {
+                m[TelemetryLogging.KeyName] = nameof(SolutionChecksumUpdater) + "." + metricName;
+                m[TelemetryLogging.KeyValue] = 1L;
+                m[TelemetryLogging.KeyMetricName] = metricName;


why do we need this here, if it is encoded in KeyName above? is one of these redundant?

This ends up down in AbstractAggregatingLog.Log. That code requires KeyName/KeyValue to be specified, and if KeyMetricName isn't specified, falls back to KeyName to be the metricName.

I can look into whether there are callers that depend on that behavior, and if not, remove that parameter. However, I'd prefer not to as even if that could be removed, it would add an allocation to strip out the metricname from the keyname.

CyrusNajmabadi · 2025-02-01T18:46:36Z

src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs

+            {
+                var client = await RemoteHostClient.TryGetClientAsync(_workspace, _shutdownToken).ConfigureAwait(false);
+                if (client == null)
+                    return false;


slightly odd that this would show up as 'failure'. this means a customer who has OOP off will show has massively failing this op. i think this should maybe count as 'success' or perhaps NA so we don't get the wrong idea about being in a bad statee.

CyrusNajmabadi · 2025-02-01T18:52:44Z

...kspaces/Remote/ServiceHub/Services/AssetSynchronization/RemoteAssetSynchronizationService.cs

+            {
+                m[TelemetryLogging.KeyName] = keyName;
+                m[TelemetryLogging.KeyValue] = 1L;
+                m[TelemetryLogging.KeyMetricName] = metricName;


same comment if we need the keyname and metric name esp if hte keyname contains the metric name.

CyrusNajmabadi · 2025-02-01T18:53:56Z

with your latest changes, i feel fine with this going in (even without Jason signing off). he will eventuallyget around to looking at this.

…every invocation and to not log telemetry in the case the RemoteHostClient.TryGetClientAsync call doesn't return a client.

ToddGrun · 2025-02-03T18:14:52Z

@sharwell has an alternative approach which he's going to try out. I'll get perf numbers on his approach once it's ready and then we can evaluate how to proceed.

In reply to: 2628571096

ToddGrun · 2025-02-04T14:10:38Z

Latest test insertion: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/607891 (based on commit 14)

…mplify the change

ToddGrun · 2025-02-05T18:03:18Z

Alternative proposed implementation by Sam here: #77059

Running speedometer tests against that and will update once those are available

jasonmalinowski · 2025-02-06T01:38:31Z

src/Workspaces/Core/Portable/Workspace/Workspace_Events.cs

+        WorkspaceChangeEventArgs args = null;
+        if (ev.HasHandlers)
+        {
+            args = new WorkspaceChangeEventArgs(kind, oldSolution, newSolution, projectId, documentId);


You could just pull this assignment out of the if() block here; practically any workspace where we would care about allocations will have at least one handler. 😄

jasonmalinowski · 2025-02-06T01:41:17Z

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/EventMap.cs

            // we won't notify all of the callers. The expectation is such an exception would be thrown to the SafeStartNew in the workspace's event queue that
            // will raise that as a fatal exception, but OperationCancelledExceptions might be able to propagate through and fault the task we are using in the
            // chain. I'm choosing to use ReportWithoutCrashAndPropagate, because if our theory here is correct, it seems the first exception isn't actually
            // causing crashes, and so if it turns out this is a very common situation I don't want to make a often-benign situation fatal.


This comment is stale now. This looks like it was written by me, but since the comment has a link to a query (rather than the actual bug), it's quite useless.

Killed the comment, things are different enough now that it's really not applicable.

jasonmalinowski · 2025-02-06T01:45:12Z

src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs

-        using var _ = ArrayBuilder<(DocumentId id, Checksum textChecksum, ImmutableArray<TextChange> changes, Checksum newTextChecksum)>.GetInstance(out var builder);
-
-        foreach (var (oldDocument, newDocument) in values)
+        _ = _threadingContext.JoinableTaskFactory.RunAsync(async () =>


I had thought GetChangeRanges could potentially end up in a text diff, but maybe not.

jasonmalinowski · 2025-02-06T01:46:04Z

src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs

+            var oldDocument = e.OldSolution.GetDocument(e.DocumentId);
+            var newDocument = e.NewSolution.GetDocument(e.DocumentId);
+
+            Debug.Assert(oldDocument != null && newDocument != null);


Suggested change

var oldDocument = e.OldSolution.GetDocument(e.DocumentId);

var newDocument = e.NewSolution.GetDocument(e.DocumentId);

Debug.Assert(oldDocument != null && newDocument != null);

var oldDocument = e.OldSolution.GetRequiredDocument(e.DocumentId);

var newDocument = e.NewSolution.GetRequiredDocument(e.DocumentId);

If we're wrong, telemetry will tell us since you're adding that catch-and-report. 😄

jasonmalinowski · 2025-02-06T01:48:24Z

src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs

+            var state = await oldDocument.State.GetStateChecksumsAsync(_shutdownToken).ConfigureAwait(false);
+            var newState = await newDocument.State.GetStateChecksumsAsync(_shutdownToken).ConfigureAwait(false);


I'm confused a bit by the semantics we're trying to have here for OnWorkspaceChangedImmediate. The statement was "it must be very fast", but now we're computing checksums? I imagine the async-ness here is we use an AsyncLazy<> under the covers, and so if somebody else is asking we won't block our thread. But this newDocument is very new (indeed, we're still raising events for it!) so would this be the first place it'll get called? And would that then be consuming CPU right here when we wanted this to be cheap?

This is a very valid concern, debugged into the hash computation for the newDocument, and this is forcing it on the calling (UI) thread. Need to think on this.

Added the task.Yield as Cyrus suggested, and verified that all the calculations have moved to a bg thread, but still saw pretty good allocation numbers as outlined in the PR updated description.

jasonmalinowski · 2025-02-06T01:49:05Z

src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs

+        // about the text changes between oldDocument and newDocument. By doing this, we can
+        // reduce the likelihood of the remote side encountering an unknown checksum and
+        // requiring a synchronization of the full document contents.
+        // This method uses JTF.RunAsync to create a fire-and-forget task. JTF.RunAsync will


Comment now stale? RunAsync not being used anymore?

Removed the stale bit.

CyrusNajmabadi · 2025-02-06T02:28:13Z

could we just do the simple thing of moving off the UI thread (use a TPL.Run or Yield), and just seeing if we still get the good results of not needing to sync anything?

ToddGrun · 2025-02-06T03:09:12Z

Yeah, maybe a Task.Yield would be good enough. Can try that out and see what the numbers say.

In reply to: 2638638591

…ount of work on the calling (UI) thread

ToddGrun · 2025-02-06T15:41:11Z

And here's the commit 17 test insertion: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/608770

Numbers looked pretty good, some serialization still happens, but it's a lot less. Will update the PR description

ToddGrun · 2025-02-09T17:21:42Z

@jasonmalinowski, @sharwell -- Will go ahead and merge on Monday unless either of you feels otherwise

ghost added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Jan 26, 2025

dotnet-policy-service bot added the VSCode label Jan 26, 2025

don't use nullability annotations

0394a81

CyrusNajmabadi reviewed Jan 26, 2025

View reviewed changes

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/FunctionId.cs Outdated Show resolved Hide resolved

Merge branch 'dotnet:main' into WorkspaceChangedImmediate

4ca3e4a

ToddGrun marked this pull request as ready for review January 27, 2025 03:36

ToddGrun requested a review from a team as a code owner January 27, 2025 03:36

ToddGrun changed the title ~~WIP: Add new workspace event that gives handlers the opportunity to be processed immediately~~ Add new workspace event that gives handlers the opportunity to be processed immediately Jan 27, 2025

Add a big old range for funciton ids that don't fit into other catego…

445cc6c

…ries

CyrusNajmabadi approved these changes Jan 27, 2025

View reviewed changes

Remove the _textChange ABWQ point of asynchronicity to try and furthe…

b1797c2

…r reduce the amount of serialization and deserialization

Switch a couple tests from FeaturesTestCompositions to EditorTestComp…

ce1b2b7

…ositions to allow composition to include an IThreadingContext

build-analysis bot mentioned this pull request Jan 27, 2025

The active test run was aborted. Reason: Test host process crashed dotnet/dnceng#451

Closed

3 tasks

push out aggregated telemetry with success/fail counts for SolutionCh…

8a841d6

…ecksumUpdater.DispatchSynchronizeTextChanges and RemoteAssetSynchronizationService.SynchronizeTextChangesAsync

CyrusNajmabadi reviewed Jan 28, 2025

View reviewed changes

src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs Outdated Show resolved Hide resolved

CyrusNajmabadi reviewed Jan 28, 2025

View reviewed changes

...kspaces/Remote/ServiceHub/Services/AssetSynchronization/RemoteAssetSynchronizationService.cs Outdated Show resolved Hide resolved

CyrusNajmabadi reviewed Jan 28, 2025

View reviewed changes

jasonmalinowski reviewed Jan 28, 2025

View reviewed changes

ToddGrun added 2 commits January 27, 2025 18:28

Comment the JTF.RunAsync call

64a596b

use some local functions for cleanup Update some comments

move duplicated code to helper method

d715ab9

CyrusNajmabadi approved these changes Jan 28, 2025

View reviewed changes

ToddGrun added 2 commits January 28, 2025 05:46

Add assert

c643d4b

Update comments Use string constants instead of allocating

Go ahead and catch exceptions during EventMap eventing

595fb9f

CyrusNajmabadi reviewed Feb 1, 2025

View reviewed changes

src/EditorFeatures/Core/Remote/SolutionChecksumUpdater.cs Outdated Show resolved Hide resolved

CyrusNajmabadi reviewed Feb 1, 2025

View reviewed changes

ToddGrun added 3 commits February 3, 2025 06:24

change DispatchSynchronizeTextChanges to not allocate the keyname on …

019e48b

…every invocation and to not log telemetry in the case the RemoteHostClient.TryGetClientAsync call doesn't return a client.

Merge branch 'main' into WorkspaceChangedImmediate

2b36d47

Fix bad merge

90003c9

just use a fire and forget task and don't use IThreadingContext to si…

d58e649

…mplify the change

jasonmalinowski reviewed Feb 6, 2025

View reviewed changes

ToddGrun added 2 commits February 5, 2025 19:44

Try out a Task.Yield to alleviate the concerns about doing a large am…

2cfcd65

…ount of work on the calling (UI) thread

Merge branch 'main' into WorkspaceChangedImmediate

5740465

ToddGrun merged commit 1adf017 into dotnet:main Feb 10, 2025
25 checks passed

dotnet-policy-service bot added this to the Next milestone Feb 10, 2025

This was referenced Feb 12, 2025

[Automated] PRs inserted in VS build main-35811.51 #77170

Closed

[Automated] PRs inserted in VS build feature.d18initial-10313.01 #77218

Closed

[Automated] PRs inserted in VS build feature.debugger.main-35813.224 #77225

Closed

333fred mentioned this pull request Feb 18, 2025

Merge main to runtime async branch #77265

Merged

akhera99 modified the milestones: Next, 17.14 P2 Feb 25, 2025

ToddGrun deleted the WorkspaceChangedImmediate branch March 5, 2025 17:16

	// 900-999 for items that don't fit into other categories.
	// 900-999 things related to workspace and OOP solution sync

		var state = await oldDocument.State.GetStateChecksumsAsync(_shutdownToken).ConfigureAwait(false);
		var newState = await newDocument.State.GetStateChecksumsAsync(_shutdownToken).ConfigureAwait(false);

Add new workspace event that gives handlers the opportunity to be processed immediately #76932

Add new workspace event that gives handlers the opportunity to be processed immediately #76932

Uh oh!

Conversation

ToddGrun commented Jan 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ToddGrun commented Jan 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

ToddGrun commented Jan 27, 2025

Uh oh!

ToddGrun commented Jan 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CyrusNajmabadi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ToddGrun commented Jan 31, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ToddGrun commented Jan 26, 2025 •

edited

Loading

ToddGrun commented Jan 26, 2025 •

edited

Loading

ToddGrun commented Jan 27, 2025 •

edited

Loading