leetvr / hotham

Hotham is a tool for creating incredible standalone VR games.
Apache License 2.0
386 stars 29 forks source link

[Feature] Hot code reloading #184

Open jmgao opened 2 years ago

jmgao commented 2 years ago

Background

Iteration time is everything. It would be really, really cool to be able to update code and have it reload in real-time

Options

TODO

kanerogers commented 2 years ago

This sounds brilliant, @jmgao! Do you've any thoughts on implementation for this?

A viable idea might be to have something like hotham-debug-runner that could watch the target directory and replace the library on build. That way, a user could just run cargo build (with the appropriate targets etc, which we could package up into a nice script, or even our own cargo command!) and the watcher service could adb push them into the app directory and restart the app.

jmgao commented 2 years ago

I did some cursory investigation, and the way Android loads the NativeActivity makes things a bit annoying to do with ndk-glue as is, because we're loading the entire library before we have a chance to load the library from another location.

I think the easiest solution is to do the following:

Now that I type this, I vaguely remember Android gaining an selinux restriction that prohibits loading libraries from application data directories if you have a sufficiently new target SDK version, but I don't remember if that happened before or after the version that the quest is on, but there are hacks around that...

jmgao commented 2 years ago

Now that I type this, I vaguely remember Android gaining an selinux restriction that prohibits loading libraries from application data directories if you have a sufficiently new target SDK version, but I don't remember if that happened before or after the version that the quest is on, but there are hacks around that...

I remembered right, but it lasted for 9 days in the tree before it was reverted.

kanerogers commented 2 years ago

I did some cursory investigation, and the way Android loads the NativeActivity makes things a bit annoying to do with ndk-glue as is, because we're loading the entire library before we have a chance to load the library from another location.

I think the easiest solution is to do the following:

  • send a patch to cargo-apk to allow us to override the library name used for the native activity in the AndroidManifest.xml
  • use that to point Android at a libhotham_loader.so, not sure what the best way to build this is
  • libhotham_loader.so loads either libraries from a fixed path in the data directory, or the lib directory, or the apk (not sure how we should decide: either bake it into libhotham_loader, or decide dynamically based on the package name? presumably we don't want to stick with rust.<name of library> as the only possible package name long term, and cargo-apk has a patch merged (but not released yet afaik) that lets you change the package name)
  • bundle a script that does a warm reload by pushing and restarting the app. I think a watcher service is overkill, given that cargo watch already exists: you can just do cargo watch -x build -s "./scripts/reload.sh"

Now that I type this, I vaguely remember Android gaining an selinux restriction that prohibits loading libraries from application data directories if you have a sufficiently new target SDK version, but I don't remember if that happened before or after the version that the quest is on, but there are hacks around that...

Oooh, that does sound like an interesting implementation.

My gut reaction is this feels like overkill and a potential source of tricky bugs. That said, if it'll give us a better developer experience, it may be worth it!

What would the indirection of adding a libhotham_loader.so give us, that couldn't be accomplished by replacing the library and restarting the app? From my experience the time to restart the app is pretty minimal, and it definitely feels like it could be a lot cleaner / simpler.

jmgao commented 2 years ago

What would the indirection of adding a libhotham_loader.so give us, that couldn't be accomplished by replacing the library and restarting the app? From my experience the time to restart the app is pretty minimal, and it definitely feels like it could be a lot cleaner / simpler.

You can't replace the library without rebuilding the APK and installing it: the application manifest says to load libcrab_saber.so, and Android will load that either from the read-only lib directory, or directly from the APK. Rebuilding the APK takes 14 seconds for me (?!?!?! why is it taking so long??) and installing takes ~4 seconds. Just pushing the libraries should be pretty much instantaneous (adb push goes at 200MB/s on my Quest 2, and that's going to get faster eventually)

We need a layer of indirection because we don't want to actually load the application library until we know which one to load, because there might be static initializers that don't expect to run twice in the same process, symbol collision if NativeActivity loads with RTLD_GLOBAL, and probably other horrible undebuggable landmines.

kanerogers commented 1 year ago

Original comment from @jmgao

It takes a pretty long time to build the libraries, create an apk and install the apk. We can improve on this by checking for libraries in a writable location and loading those over the ones in the apk if found. Then we can build just the libraries, push those to the app data directory and then restart the app.

(I think we can improve on this further by making hotham a separate shared library from the actual application, but the cargo incantations for that might be painful)