Skip to content
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

Reworked backend events #2402

Merged
merged 3 commits into from
Dec 13, 2023
Merged

Conversation

RunDevelopment
Copy link
Member

Fixes #2319

This PR reworks the events the executor sends to the frontend. I changed both, what events are send, how they are sent, the data the events contains, and how and in which order the events are processed by the frontend. The frontend now also keeps track of the overall progress of the chain instead of the backend like before.

Let's start with the backend changes:

  • Added chain-start event. This event contains the list of nodes that will actually be executed. Due to caching and optimizations, this list is often different from the list of nodes the frontend sent to the backend. This list will be used for animations and progress by the frontend.
  • Moved broadcasts into its own event. This is important for iterators where we do one broadcast for each iteration.
  • Move all event code in the executor into __send_* methods. This makes the code pretty clean.
  • Fixed a bug in the executor whether broadcasts would be sent out even after the execution as aborted.
  • Other minor improvements to the executor.
  • Removed useless message field in some responses of the backend API.

Frontend changes:

  • Running individual nodes no longer uses the animate/unAnimate methods like regular executions. They now have their own state to make sure these two systems don't interfere with each other.
  • Edges now derive their animation state from their connected source node. This is not completely correct yet (discussion on discord), but will be soon.
  • Iterators now do type narrowing for the current iteration. This means that they display e.g. the image name of the current iteration. I was careful not to re-introduce Iterator broadcast persistence can break Fill Alpha #838 here. The type narrowing is always removed after the execution complete (successfully or otherwise).
  • Overall chain progress is now tracked entirely by the frontend. I implemented the weighted node progress idea I proposed some time ago, but weights aren't assigned in a clever way yet. This should be easy to improve in follow-up PRs. That being said, it's still better that what we had before because it takes iterator progress into account.
  • All node events are now processed sequentially. We previously batched events to process them more efficiently. However, the batching was done per event type, which caused some events to be executed out of order (e.g. a node-finish might be run before a node-start). This was the cause of the animation keeps playing #2319, and I fixed it by batching all node events together. I made the batch queue explicit and called it a backlog. It's necessary for the backlog to be explicit because we have to wait for all events to be fully processed before we can declare a run to be truly finished, or else some nasty race conditions occur.
    The backlog also has the advantage that we can process the whole queue at once instead of individual events, which allows for some nice optimizations when there are many events (e.g. a fast iterator).

@RunDevelopment
Copy link
Member Author

As discussed on discord, I also added a new system for keeping track of the execution status of a node (not yet, running, finished, not executing). This fixes the issue with edges animating that I mentioned above.

I moved the state itself into the ExecutionContext because nothing in the global node context needed it.

Copy link
Member

@joeyballentine joeyballentine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. 👍

@joeyballentine joeyballentine merged commit 81d8fe3 into chaiNNer-org:main Dec 13, 2023
17 checks passed
@RunDevelopment RunDevelopment deleted the event-rework branch December 13, 2023 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

the animation keeps playing
2 participants