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

Reconfigured configuration reloads to use restarts. #1456

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

adamharrison
Copy link
Member

Do not merge this until/unless #1455 is merged; this PR is based off that branch, and will be rebased as necessary, as that PR significantly decreases startup times in certain circumstances.

Exploratory.

We've had a number of issues over the years with our current system of reloading. Normally, when you reload a module, it works, sometimes. But sometimes it doesn't; depending on exactly what's being reloaded, and where.

This causes all sorts of problems with inconsistent behaviour across restarting vs. reloading.

The solution we've talked about in the past was simply doing away with reloading, and focusing on restarting. This has a number of advantages.

  1. It allows for consistent behaviour across the board, with no distinctions between reloading and restarting.
  2. Certain things, which couldn't be done before (properly re-overloading functions) are now done efficiently and correctly at all times; the current model of override never really worked with the reload system.
  3. It removes a large chunk of code that deals with this dichotomy and simplifies the system greatly.
  4. It forces a high-quality reload experience, since we're no longer jumping between reloads and restarts; for things to be submitted and work, they must restart correctly, which should've always been the case anyway.
  5. It eliminates bugs from modules that don't ever expect to be reloaded and aren't coded for it.

It also has a disadvantage or two:

  1. In theory, plugins that have long running processes have no way of persisting between restarts easily without calling detach.
  2. Plugins that have slow-startup times will be unnecessarily reloaded, and could slow things down on a restart.

IMO, the advantages by far and away outweigh the disadvantages. This PR is going to focus on making sure it all works, while proposing enhancements to restarting, so that it's as seamless and smooth as possible. WIP.

@adamharrison
Copy link
Member Author

OK; so it's now pretty seamless for restarts, which is pretty cool. It's not quite as fast as the reload_module of course; but its' pretty damn close. Gonna see what I can do to make it start even faster.

@adamharrison
Copy link
Member Author

adamharrison commented Apr 4, 2023

OK, did some profiling. Currently, on my machine, a restart, until end of first new frame takes ~180ms.

Further breaking it down, it's about 100ms to restart, 80ms to draw the first frame. 35ms of that is rendering fonts into the font atlas.

I have a feeling we can definitely increase the speed here. But I'll have to think about it. I want to avoid doing anything complicated like dumping the pre-compiled chunks or something, unless that can be achieved extremely easily.

Will have to think about this. But as it stands, I think 180ms is perfectly acceptable. Under 100ms would be nice, and under 50ms would be magical, but as it stands, unless you're doing major graphical redraws (i.e. changing the style), the restart is basically not noticeable, and it is in fact hard to tell that you just performed a restart.

@adamharrison adamharrison marked this pull request as ready for review April 10, 2023 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant