Add support for pinning actors to a dedicated scheduler thread #4547

dipinhora · 2024-11-24T17:52:17Z

The overall design goal and approach was to make it possible to have pinned actors while minimizing impact to pre-existing non-pinned actor workloads. This meant there could be no impact on message sends (i.e. can't check to see if the receiving actor is a pinned actor or not to decide what to do with it if it is unscheduled). These goals were chosen because it is expected that pinned actors will be a niche/small part of any pony application's overall workload.

The approach taken has negligible performance impact to existing scheduler logic. It adds a couple of extra checks to see if an actor that is ready to run is a pinned actor or not and if not, there is no other overhead involved. The scheduler quiescence logic has an extra check for an atomic counter of pinned actors but that is also negligible if no pinned actors are ever used.

The overall logic for pinned actors works as follows:

The main thread is dedicated to running pinned actors (and only pinned actors). This thread previously initialized the runtime and then sat around waiting for all schedulers to reach quiescence so now it runs pinned actors in the meantime if there are any.
The pinned actor thread (main) runs a custom run loop for pinned actors that does not participate in work stealing or any other normal scheduler messaging except for unmuting messages and the termination message. It also will only ever run pinned actors and any non-pinned actors will get pushed onto the inject queue.
Normal schedulers will only ever run non-pinned actors and any pinned actors will get pushed onto the pinned actor thread's queue.
From an api perspective, there is now an actor_pinning package in the stdlib. An actor can request to be pinned, check that it has successfully been pinned (so that it can safely do whatever it needs to do while pinned), and request to be unpinned.

While the above is not necessarily the most efficient way to run pinned actors, it meets the original design goals of making it possible while minimizing impact of pre-existing non-pinned actor workloads.

SeanTAllen · 2024-11-24T18:31:25Z

We should discuss if this should have anything added to the tutorial to cover this (and other scheduler basics).

ponylang-main · 2024-11-24T18:32:34Z

Hi @dipinhora,

The changelog - added label was added to this pull request; all PRs with a changelog label need to have release notes included as part of the PR. If you haven't added release notes already, please do.

Release notes are added by creating a uniquely named file in the .release-notes directory. We suggest you call the file 4547.md to match the number of this pull request.

The basic format of the release notes (using markdown) should be:

## Title

End user description of changes, why it's important,
problems it solves etc.

If a breaking change, make sure to include 1 or more
examples what code would look like prior to this change
and how to update it to work after this change.

Thanks.

dipinhora · 2024-11-24T18:36:23Z

We should discuss if this should have anything added to the tutorial to cover this (and other scheduler basics).

yeah, that's probably a good idea... also, not sure if this should be experimental or not initially..

SeanTAllen · 2024-11-24T18:40:41Z

We should discuss if this should have anything added to the tutorial to cover this (and other scheduler basics).

yeah, that's probably a good idea... also, not sure if this should be experimental or not initially..

what would "experimental" mean in practical purposes? behind a runtime flag?

dipinhora · 2024-11-24T18:45:18Z

We should discuss if this should have anything added to the tutorial to cover this (and other scheduler basics).

yeah, that's probably a good idea... also, not sure if this should be experimental or not initially..

what would "experimental" mean in practical purposes? behind a runtime flag?

erm... unstable/might change as folks use it and give feedback or similar? 8*/

not necessarily behind a runtime flag since it's effectively a no-op if no actors ever get pinned..

dipinhora · 2024-11-24T20:44:30Z

not sure why PR / x86-64 Apple Darwin (pull_request) is unhappy but it failed with:

/Users/runner/work/ponyc/ponyc/src/libponyrt/asio/kqueue.c:295: pony_asio_event_subscribe: Assertion `b != NULL` failed.

and nothing in this PR should changed how ASIO shutdown coordination occurs to possibly trigger that assertion..

SeanTAllen · 2024-11-25T13:13:41Z

not sure why PR / x86-64 Apple Darwin (pull_request) is unhappy but it failed with:
/Users/runner/work/ponyc/ponyc/src/libponyrt/asio/kqueue.c:295: pony_asio_event_subscribe: Assertion `b != NULL` failed.
and nothing in this PR should changed how ASIO shutdown coordination occurs to possibly trigger that assertion..

I think there's a race condition in shutdown with kqueue and other asio systems and that a recent change made it more likely to happen in our CI.

UPDATE: I found an issue and opened a PR: #4548

packages/actor_pinning/actor_pinning.pony

packages/actor_pinning/auth.pony

dipinhora · 2024-11-25T16:01:31Z

release notes added

dipinhora · 2024-11-25T16:40:35Z

not sure why PR / x86-64 Apple Darwin (pull_request) is unhappy but it failed with:
/Users/runner/work/ponyc/ponyc/src/libponyrt/asio/kqueue.c:295: pony_asio_event_subscribe: Assertion `b != NULL` failed.
and nothing in this PR should changed how ASIO shutdown coordination occurs to possibly trigger that assertion..
I think there's a race condition in shutdown with kqueue and other asio systems and that a recent change made it more likely to happen in our CI.

UPDATE: I found an issue and opened a PR: #4548

wow.. it happened for 3 more CI builds.. 8*/

maybe i should rebase onto main to get your fix?

SeanTAllen · 2024-11-25T17:04:07Z

not sure why PR / x86-64 Apple Darwin (pull_request) is unhappy but it failed with:
/Users/runner/work/ponyc/ponyc/src/libponyrt/asio/kqueue.c:295: pony_asio_event_subscribe: Assertion `b != NULL` failed.
and nothing in this PR should changed how ASIO shutdown coordination occurs to possibly trigger that assertion..
I think there's a race condition in shutdown with kqueue and other asio systems and that a recent change made it more likely to happen in our CI.
UPDATE: I found an issue and opened a PR: #4548
wow.. it happened for 3 more CI builds.. 8*/

maybe i should rebase onto main to get your fix?

No guarantee it fixes, but yes, sounds like a good idea.

SeanTAllen · 2024-11-25T21:08:47Z

@dipinhora thoughts on main thread vs starting a dedicated thread for pinned actors and leaving the main thread "simpler"?

packages/actor_pinning/actor_pinning.pony

dipinhora · 2024-11-25T21:23:26Z

@dipinhora thoughts on main thread vs starting a dedicated thread for pinned actors and leaving the main thread "simpler"?

i have no preference one way or the other but my limited understanding is that some libraries/OSes require that the main thread be the one that is used for windowing/graphics type things so i used the main thread to ensure those use cases are also supported..

SeanTAllen · 2024-11-25T22:06:02Z

It would be nice to be able to pin the "pinned actor" thread like we can with the asio thread and the ponypinasio option does.

I'm not sure how that would interact with having the main startup thread be the "pinned actor" thread.

dipinhora · 2024-11-26T00:37:07Z

not sure why PR / x86-64 Apple Darwin (pull_request) is unhappy but it failed with:
/Users/runner/work/ponyc/ponyc/src/libponyrt/asio/kqueue.c:295: pony_asio_event_subscribe: Assertion `b != NULL` failed.
and nothing in this PR should changed how ASIO shutdown coordination occurs to possibly trigger that assertion..
I think there's a race condition in shutdown with kqueue and other asio systems and that a recent change made it more likely to happen in our CI.
UPDATE: I found an issue and opened a PR: #4548
wow.. it happened for 3 more CI builds.. 8*/
maybe i should rebase onto main to get your fix?
No guarantee it fixes, but yes, sounds like a good idea.

force pushed after the rebase.. hopefully it helps..

dipinhora · 2024-11-26T00:38:00Z

It would be nice to be able to pin the "pinned actor" thread like we can with the asio thread and the ponypinasio option does.

sure would! 8*P

I'm not sure how that would interact with having the main startup thread be the "pinned actor" thread.

me neither.. i'm going to punt on pinning the pinned actor thread as a follow up thing to do outside of this PR..

SeanTAllen · 2024-11-26T01:58:47Z

not sure why PR / x86-64 Apple Darwin (pull_request) is unhappy but it failed with:
/Users/runner/work/ponyc/ponyc/src/libponyrt/asio/kqueue.c:295: pony_asio_event_subscribe: Assertion `b != NULL` failed.
and nothing in this PR should changed how ASIO shutdown coordination occurs to possibly trigger that assertion..
I think there's a race condition in shutdown with kqueue and other asio systems and that a recent change made it more likely to happen in our CI.
UPDATE: I found an issue and opened a PR: #4548
wow.. it happened for 3 more CI builds.. 8*/
maybe i should rebase onto main to get your fix?
No guarantee it fixes, but yes, sounds like a good idea.
force pushed after the rebase.. hopefully it helps..

Looks like there is still an issue. Null pointer boom in the same place with a release build. Huh.

dipinhora · 2024-11-26T03:20:47Z

Looks like there is still an issue. Null pointer boom in the same place with a release build. Huh.

that's annoying..

a recent change made it more likely to happen in our CI

any idea what change made the ASIO backend null assertion trigger more often in CI? maybe that'll provide a clue that might help identify the root cause..

SeanTAllen · 2024-11-26T03:55:02Z

Looks like there is still an issue. Null pointer boom in the same place with a release build. Huh.

that's annoying..

a recent change made it more likely to happen in our CI

any idea what change made the ASIO backend null assertion trigger more often in CI? maybe that'll provide a clue that might help identify the root cause..

it is possible on startup for the backend to be set to NULL and for us to continue running so that is also a possibility.

SeanTAllen · 2024-11-26T04:19:39Z

Looks like there is still an issue. Null pointer boom in the same place with a release build. Huh.

that's annoying..

a recent change made it more likely to happen in our CI

any idea what change made the ASIO backend null assertion trigger more often in CI? maybe that'll provide a clue that might help identify the root cause..

it is possible on startup for the backend to be set to NULL and for us to continue running so that is also a possibility.

actually i cant see how we would keep running if it is null.

dipinhora · 2024-11-26T04:23:16Z

i think i know what's going on with the ASIO null assertion issue.. the shutdown logic in scheduler.c at

ponyc/src/libponyrt/sched/scheduler.c

Line 477 in f0d7afd

if(sched->ack_count >= current_active_scheduler_count)

is borked when scheduler scaling is enabled and as a result the ASIO thread is being shutdown too early..

SeanTAllen · 2024-11-26T04:46:12Z

i think i know what's going on with the ASIO null assertion issue.. the shutdown logic in scheduler.c at

ponyc/src/libponyrt/sched/scheduler.c

Line 477 in f0d7afd

if(sched->ack_count >= current_active_scheduler_count)

is borked when scheduler scaling is enabled and as a result the ASIO thread is being shutdown too early..

I've been looking for a way for the shutdown to happen early. That is probably another source of bug. If it shuts down early, the backend would be set to NULL while we are still trying to use it.

What's the proper logic? Can you open a PR?

SeanTAllen · 2024-11-26T19:16:54Z

src/libponyrt/sched/scheduler.c

@@ -470,7 +575,8 @@ static bool quiescent(scheduler_t* sched, uint64_t tsc, uint64_t tsc2)

  // only scheduler 0 can initiate shutdown (it is the ony that gets all the
  // ACK messages as part of the CNF/ACK coordination for shutdown)
-  if(0 == sched->index)
+  // only if there are no pinned actors in the system...
+  if(0 == sched->index && 0 == get_pinned_actor_count())


why the check against the pinned actor count for quiescence?

so an app with only pinned actors doesn't shut down too early..

pinning doesn't prevent garbage collection of the actor so, i don't think this is needed.

i think what is needed is the usual "there's a noisy" or similiar.

we discussed this a good deal on the sync call. let's find some time to have a synchronous conversation, you and i about this.

pinning doesn't prevent garbage collection of the actor so, i don't think this is needed.

i don't see how garbage collection of pinned actors is related to this..

i think what is needed is the usual "there's a noisy" or similiar.

sure, that's another implementation option.. the current option falls into or similar in my opinion because i expect the atomic variable used would have little to no contention so that's what i implemented..

we discussed this a good deal on the sync call. let's find some time to have a synchronous conversation, you and i about this.

yep.. sounds like a plan..

why would an app with only pinned actors "shutdown early"?

no.. my test was without any pinned actors.. your statement indicated that noblock would prevent quiescent if there are any cycles even with no pinned actors (or i misunderstood your statement to imply this) and i wanted to confirm that behavior...

I was incorrect on that front.

no.. my test was without any pinned actors..

So this change, without any actors pinned causes the ring example to not exit if noblock is used?

no.. my test was without any pinned actors..

So this change, without any actors pinned causes the ring example to not exit if noblock is used?

no, this change doesn't impact pre-existing behavior is no actors are pinned.. the quiescence logic is impacted only if actors are pinned...

what part do you think is problematic? the existing behavior or the change in behavior with this PR and pinned actors or both?

I think the ideal would be for you to be able to pin actors and have everything "continue to work as before". Whether this is doable with the various trade-offs is what I want to look at.

yes, we discussed this already in comments yesterday and landed on experimental for now while we iterate on the quiescence implementation details in future PRs as opposed to trying to be perfect with this PR..

what part do you think is problematic? the existing behavior or the change in behavior with this PR and pinned actors or both?

I think the ideal would be for you to be able to pin actors and have everything "continue to work as before". Whether this is doable with the various trade-offs is what I want to look at.

yes, we discussed this already in comments yesterday and landed on experimental for now while we iterate on the quiescence implementation details in future PRs as opposed to trying to be perfect with this PR..

yup. i'm ok with that approach. i am trying to understand better to craft what i think would be a more clear "caveat" message.

dipinhora · 2024-11-26T21:38:48Z

i think i know what's going on with the ASIO null assertion issue.. the shutdown logic in scheduler.c at

ponyc/src/libponyrt/sched/scheduler.c

Line 477 in f0d7afd

if(sched->ack_count >= current_active_scheduler_count)

is borked when scheduler scaling is enabled and as a result the ASIO thread is being shutdown too early..

I've been looking for a way for the shutdown to happen early. That is probably another source of bug. If it shuts down early, the backend would be set to NULL while we are still trying to use it.

What's the proper logic? Can you open a PR?

just opened #4550

SeanTAllen · 2024-11-27T17:13:29Z

@dipinhora once #4553 is merged, please rebase this against main.

SeanTAllen · 2024-11-28T17:16:03Z

.release-notes/4547.md

+
+The way it works is that an actor can request that it be pinned (which may or may not happen immediately) and then it must wait and check to confirm that the pinning was successfully applied (prior to running any workload that required the actor to be pinned) after which all subsequent behaviors on that actor will run on the same scheduler thread until the actor is destroyed or the actor requests to be unpinned.
+
+Additional details can be found in the `actor_pinning` package in the standard library.


I like "robust" release notes. Can you add an example program in here that demonstrates the concepts?

I also think the release notes should include a notice that this is an experimental feature and likely to change. That the implementation will evolve and that might change the feature in general. That sort of thing.

the release notes have been fattened up..

packages/actor_pinning/actor_pinning.pony

redvers · 2024-11-29T02:57:02Z

I've been ~~torturing~~ exercising this PR over the last day or so and it is incredibly consistent.

I did both extremes:

I filled all the (regular) schedulers with busy actors to force contention.
I flooded the runtime with pinned actors and nothing else.

In all cases, it moved the pinned actors 100% of the time on the first behaviour call.

I have yet to break it.

dipinhora · 2024-11-29T05:03:12Z

In all cases, it moved the pinned actors 100% of the time on the first behaviour call.

This is a function of the program design..

the following takes 4 attempts before pinning is finally successful:

use "actor_pinning"

actor Main
  new create(env: Env) =>
    let p = Pactor(env)
    // the following all happen before the actor is pinned
    p.check_pinned()
    p.check_pinned()
    p.check_pinned()

actor Pactor
  let _env: Env
  let _auth: PinUnpinActorAuth
  var _count: I32 = 1

  new create(env: Env) =>
    _env = env
    _auth = PinUnpinActorAuth(env.root)
    ActorPinning.request_pin(_auth)
    // the following will happen after the actor is pinned
    check_pinned()

  be check_pinned() =>
    if ActorPinning.is_successfully_pinned(_auth) then
      _env.out.print("Attempts until successfully pinned: " + _count.string())
    else
      _count = _count + 1
    end

while the following is successful on the first attempt:

use "actor_pinning"

actor Main
  new create(env: Env) =>
    let p = Pactor(env)

actor Pactor
  let _env: Env
  let _auth: PinUnpinActorAuth
  var _count: I32 = 1

  new create(env: Env) =>
    _env = env
    _auth = PinUnpinActorAuth(env.root)
    ActorPinning.request_pin(_auth)
    // the following will happen after the actor is pinned
    check_pinned()

  be check_pinned() =>
    if ActorPinning.is_successfully_pinned(_auth) then
      _env.out.print("Attempts until successfully pinned: " + _count.string())
    else
      _count = _count + 1
    end

SeanTAllen · 2024-11-29T14:20:27Z

In all cases, it moved the pinned actors 100% of the time on the first behaviour call.

This isn't a meaningful statement. As Dipin demonstrates, it has nothing to do with the feature and everything to do with when you are checking. If you put a check in a loop immediately after requesting a pin then you'd never see success based on the current runtime implementation.

The important bit is "it will happen eventually" where "eventually" is implementation specific.

dipinhora · 2024-12-01T04:01:55Z

#4556 has been opened for more shutdown/termination logic changes to try and get that edge case that kept cropping up in the arm64 apple darwin builds (and sometimes also in other builds)..

it would probably make sense to wait for it to be merged and then to rebase this PR onto main to pick up the changes before merging this one..

SeanTAllen · 2024-12-03T19:57:48Z

@dipinhora we are good with going forward with the "experimental" strategy we discussed.

SeanTAllen · 2024-12-04T20:33:30Z

@dipinhora you can rebase this against main. the quiescence fix is in.

The overall design goal and approach was to make it possible to have pinned actors while minimizing impact to pre-existing non-pinned actor workloads. This meant there could be no impact on message sends (i.e. can't check to see if the receiving actor is a pinned actor or not to decide what to do with it if it is unscheduled). These goals were chosen because it is expected that `pinned` actors will be a niche/small part of any pony application's overall workload. The approach taken has negligible performance impact to existing scheduler logic. It adds a couple of extra checks to see if an actor that is ready to run is a pinned actor or not and if not, there is no other overhead involved. The scheduler quiescence logic has an extra check for an atomic counter of pinned actors but that is also negligible if no pinned actors are ever used. The overall logic for pinned actors works as follows: * The `main` thread is dedicated to running pinned actors (and only pinned actors). This thread previously initialized the runtime and then sat around waiting for all schedulers to reach quiescence so now it runs pinned actors in the meantime if there are any. * The `pinned actor thread` (`main`) runs a custom run loop for pinned actors that does not participate in work stealing or any other normal scheduler messaging except for unmuting messages and the termination message. It also will only ever run `pinned` actors and any non-`pinned` actors will get pushed onto the `inject` queue. * Normal schedulers will only ever run non-`pinned` actors and any `pinned` actors will get pushed onto the `pinned actor thread`'s queue. * From an api perspective, there is now an `actor_pinning` package in the stdlib. An actor can request to be pinned, check that it has successfully been pinned (so that it can safely do whatever it needs to do while pinned), and request to be unpinned. While the above is not necessarily the most efficient way to run `pinned` actors, it meets the original design goals of making it possible while minimizing impact of pre-existing non-pinned actor workloads.

and also update the package documentation to clarify that `pinning` is not an immediate action

Co-authored-by: Sean T Allen <sean@seantallen.com>

dipinhora · 2024-12-04T21:17:24Z

@dipinhora you can rebase this against main. the quiescence fix is in.

rebased..

@dipinhora we are good with going forward with the "experimental" strategy we discussed.

ummm.. about that... the musl detour during the #4556 sidequest left me some time so the pinned actor thread now participates in the CNF/ACK termination process..

It would be nice to be able to pin the "pinned actor" thread like we can with the asio thread and the ponypinasio option does.

sure would! 8*P

also for the same reason, the pinned actor thread can be pinned now..

SeanTAllen · 2024-12-05T01:45:10Z

src/libponyrt/options/options.h

@@ -43,6 +43,10 @@
  "  --ponypinasio    Pin the ASIO thread to a CPU the way scheduler\n" \
  "                   threads are pinned to CPUs. Requires `--ponypin` to\n" \
  "                   be set to have any effect.\n" \
+  "  --ponypinpinnedactorthread\n" \


sweet addition

ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Nov 24, 2024

SeanTAllen added the changelog - added Automatically add "Added" CHANGELOG entry on merge label Nov 24, 2024

SeanTAllen reviewed Nov 25, 2024

View reviewed changes

packages/actor_pinning/actor_pinning.pony Outdated Show resolved Hide resolved

SeanTAllen reviewed Nov 25, 2024

View reviewed changes

packages/actor_pinning/auth.pony Show resolved Hide resolved

SeanTAllen reviewed Nov 25, 2024

View reviewed changes

packages/actor_pinning/actor_pinning.pony Show resolved Hide resolved

dipinhora force-pushed the pinned_actors branch from bddf568 to d8a07e0 Compare November 26, 2024 00:36

SeanTAllen reviewed Nov 26, 2024

View reviewed changes

dipinhora force-pushed the pinned_actors branch from d5f6321 to a21209d Compare November 28, 2024 01:34

SeanTAllen reviewed Nov 28, 2024

View reviewed changes

packages/actor_pinning/actor_pinning.pony Outdated Show resolved Hide resolved

dipinhora and others added 13 commits December 4, 2024 16:07

Fix unused variable error

a8bd362

Fix dtrace undeclared variable error

630e707

Fix windows pinned-actor test linking error

9f2ffc9

Add greedy actor caveat to actor pinning package

450bfde

Add release notes

334d998

Rename pin to request_pin and unpin to request_unpin

80aa098

and also update the package documentation to clarify that `pinning` is not an immediate action

update release notes

0f787fa

Use correct function names now that they've been renamed

189151b

update caveat

76a48fa

Co-authored-by: Sean T Allen <sean@seantallen.com>

Fatten up the release notes

417b55d

make pinned actor thread participate in CNF/ACK for termination

8c338ae

add option to pin pinned actor thread

2dfa7d7

dipinhora force-pushed the pinned_actors branch from 4c5032f to 2dfa7d7 Compare December 4, 2024 21:16

SeanTAllen reviewed Dec 5, 2024

View reviewed changes

SeanTAllen approved these changes Dec 5, 2024

View reviewed changes

SeanTAllen merged commit 9d178ed into ponylang:main Dec 5, 2024
23 checks passed

ponylang-main removed the discuss during sync Should be discussed during an upcoming sync label Dec 5, 2024

github-actions bot pushed a commit that referenced this pull request Dec 5, 2024

Update CHANGELOG for PR #4547

2a502af

github-actions bot pushed a commit that referenced this pull request Dec 5, 2024

Updates release notes for PR #4547

2246089


		The way it works is that an actor can request that it be pinned (which may or may not happen immediately) and then it must wait and check to confirm that the pinning was successfully applied (prior to running any workload that required the actor to be pinned) after which all subsequent behaviors on that actor will run on the same scheduler thread until the actor is destroyed or the actor requests to be unpinned.

		Additional details can be found in the `actor_pinning` package in the standard library.

Add support for pinning actors to a dedicated scheduler thread #4547

Add support for pinning actors to a dedicated scheduler thread #4547

Conversation

dipinhora commented Nov 24, 2024

SeanTAllen commented Nov 24, 2024

ponylang-main commented Nov 24, 2024

dipinhora commented Nov 24, 2024

SeanTAllen commented Nov 24, 2024

dipinhora commented Nov 24, 2024

dipinhora commented Nov 24, 2024

SeanTAllen commented Nov 25, 2024 • edited Loading

dipinhora commented Nov 25, 2024

dipinhora commented Nov 25, 2024

SeanTAllen commented Nov 25, 2024 • edited Loading

SeanTAllen commented Nov 25, 2024

dipinhora commented Nov 25, 2024

SeanTAllen commented Nov 25, 2024

dipinhora commented Nov 26, 2024

dipinhora commented Nov 26, 2024

SeanTAllen commented Nov 26, 2024

dipinhora commented Nov 26, 2024

SeanTAllen commented Nov 26, 2024

SeanTAllen commented Nov 26, 2024

dipinhora commented Nov 26, 2024

SeanTAllen commented Nov 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dipinhora Nov 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dipinhora commented Nov 26, 2024

SeanTAllen commented Nov 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

redvers commented Nov 29, 2024

dipinhora commented Nov 29, 2024

SeanTAllen commented Nov 29, 2024

dipinhora commented Dec 1, 2024

SeanTAllen commented Dec 3, 2024

SeanTAllen commented Dec 4, 2024

dipinhora commented Dec 4, 2024

Choose a reason for hiding this comment

SeanTAllen commented Nov 25, 2024 •

edited

Loading

SeanTAllen commented Nov 25, 2024 •

edited

Loading

SeanTAllen commented Nov 26, 2024 •

edited

Loading

dipinhora Nov 26, 2024 •

edited

Loading