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

Rewrite the main loop for simplicity and perf #108

Merged
merged 24 commits into from
Jan 24, 2022
Merged

Conversation

dhleong
Copy link
Owner

@dhleong dhleong commented Jan 23, 2022

This is a massive refactor that consolidates all the many ways we have of getting onto the main thread into a single Dispatcher abstraction. In particular:

  • The main loop is no longer a loop around many tiny sleeps, and instead is exclusively a "wait" on a channel, which should overall reduce power usage since the vast majority of time in a MUD is probably waiting for something to happen (either output from the server, or input from the user).
  • Connections are rewritten to be multi-threaded
  • The "Connection" abstraction was renamed to "Transport," which better describes how it is used
  • Jobs still exist as a means of supporting multi-threaded work, but they access the main thread through Dispatcher (including support for results!)
  • Scripts (of course) also access the main thread via Dispatcher
  • Main thread dispatches are handled in batches; if multiple come in at once, we consistently handle them all so long as we don't have to do a blocking read to get them, and the elapsed processing time does not exceed one "frame" of time.
  • Complete removed the ApiManagerDelegate since we no longer need it; this solves the discussion around re-entrant scripting since we don't have to remove anything from app::State to respond to scripting methods.

May just be placebo, but things do feel a bit snappier now!

Future work might:

  • Rewrite Telnet support using on Tokio's async TCP connections
  • Refactor Dispatcher to receive an Enum so we can provide an explicit "There's a pending key" signal that shortcuts out of whatever it's doing
  • Refactor Connections to not be Option on app::State; not strictly necessary (or very relevant to this PR) but also I don't think we need it, and that refactor would certainly simplify access to connections

We now relentlessly enqueue buffer appends to the main thread. It's
certainly not as efficient as batching all the writes in some interval,
but in practice it shouldn't be a problem. In particular, we no longer
for a write to complete before beginning the next read, and we also
don't wait for data on the wire before continuing with data we have
buffered.
Still need to make sure the GameConnection processes everything
correctly, and possibly rename it to GameTransport
Fixes issue where you could not delete a connection buffer after the
remote side disconnected
We still hang when attempting to send something, however.... Not quite
sure why yet
There is now only one way to get onto the main thread!
Should keep CPU usage more consistently low due to more consistent
sleep. We basically use the Dispatcher to send a no-op fn to the main
thread as a signal that the UiEvents can be polled.

We can probably simplify further and possibly process more than a
single dispatched event per loop (as we had been doing).
@dhleong dhleong merged commit 4120e21 into main Jan 24, 2022
@dhleong dhleong deleted the dhleong/new-main branch January 24, 2022 15:26
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.

1 participant