Build libultraship into the binary directory and the host asset packer optimized - #418
Conversation
ed9d77c to
79fd851
Compare
…r optimised Two independent build fixes to the top-level CMakeLists.txt. The second argument to add_subdirectory() is the *binary* directory, but it was pointed at the source directory. That made libultraship write its entire build output -- CMakeScripts, cmake_install.cmake, generated headers and every target's .a -- into the submodule's source tree, so every build directory shared one output location. Two builds running at once would overwrite each other's artifacts, and `git status` inside the submodule was permanently dirty. Dropping the argument lets CMake place the output under the binary directory as usual. Note that this relocates libultraship's build output: anyone with scripts or tooling that reference build artefacts inside the libultraship/ source tree will need to update those paths. Separately, TorchExternal was being built without optimisation. ExternalProject does not inherit CMAKE_BUILD_TYPE, and an empty build type on a single-config generator -- Ninja or Makefiles, which is the normal Linux and macOS command-line path -- means no optimisation flags at all, so the packer was compiled -O0 even from a Release tree. Torch is a build-time asset packer, so it is now configured Release explicitly regardless of the host project's configuration. Multi-config generators such as Visual Studio and Xcode ignore CMAKE_BUILD_TYPE at configure time and are unaffected either way. The cost was not subtle. Extracting bk.o2r from a 16 MB ROM on the same machine took 2312.96 s (38.5 minutes) unoptimised versus 38.28 s at -O3, roughly 60x, for byte-identical 24 MB output. The work is single-threaded and 99.8% user CPU, so this was never I/O or memory bound.
79fd851 to
ba5e097
Compare
|
I tried implementing the Torch thing locally, and it was a no-op. If anything, it added 10-15 seconds to my extraction time compared to previous benchmarks. This is on Windows. Also, I don't think removing that second argument to |
Omitting the second argument to add_subdirectory() already resolves to
${CMAKE_CURRENT_BINARY_DIR}/libultraship, so this is not a behavior change --
it just states the destination rather than relying on the default.
|
That tracks. VS is multi-config, and Configuring Torch both ways with Ninja, Windows Debug builds do still get a Debug Torch, though, since On |
Mirrors the change made on lh/cmake-build-fixes for PR HarbourMasters#418. Omitting the second argument to add_subdirectory() already resolves to ${CMAKE_CURRENT_BINARY_DIR}/libultraship, so this is not a behavior change.
|
Any ideas, then, on why it still takes 1:30 on Windows in release config, if Mac is less than 40 with this now? |
|
Honestly, no clue why it's different on Windows. I'm working on an Ubuntu PC and a MacBook Pro with an ARM processor. The best test I can offer is to build twice and make 2 separate builds; with and without the change. Then swap back and forth a few times as a test. This makes both tests function identically. The extraction is single threaded so there's a small chance you're getting CPU throttled post compile. That said there really shouldn't be any delta for Windows. What I do know is on Linux, Mac, and especially iOS devices the change is enormous before and after. It still extracts on Apple hardware but it's a slog. |
Two independent, small fixes to the top-level
CMakeLists.txt. Both are one-linechanges plus explanatory comments.
Asset extraction is ~60x faster on MacOS and iOS
When I was developing my own port to iOS the in-game MacOS and iPad on-device extraction to
bk.o2rfrom a 16 MB ROM, same machine, same input:-O3(this PR)Byte-identical 24 MB output. The work is single-threaded and 99.8% user CPU, so
this was never I/O or memory bound — it was simply an un-optimized binary.
These numbers were measured on macOS while tracking down why on-device asset
extraction was taking tens of minutes; they have not been re-measured on Linux or
Windows. The absolute times will differ per machine, but the cause is generator-
level and not platform-specific, so any single-config generator build should see
the same class of improvement.
Why it was happening
ExternalProjectdoes not inheritCMAKE_BUILD_TYPEfrom the parent project, andTorchExternalnever passed one. On a single-config generator — Ninja orMakefiles, which is the normal Linux and macOS command-line path — an empty build
type means no optimization flags at all, so the asset packer was compiled
-O0even when the surrounding project was configured
Release.Torch is a build-time tool, so the fix is to configure it
Releaseunconditionallyrather than trying to track the host project's configuration:
Multi-config generators (Visual Studio, Xcode) ignore
CMAKE_BUILD_TYPEatconfigure time and are unaffected either way. The
if (CMAKE_CONFIGURATION_TYPES)test that selects
TORCH_EXECUTABLEis deliberately left untouched.libultraship built into the binary directory
The second argument to
add_subdirectory()is the binary directory, not a secondsource path. Pointing it at the source directory made libultraship write its entire
build output —
CMakeScripts,cmake_install.cmake, generated headers and everytarget's
.a— into the submodule's source tree. Consequences:overwrote each other's artifacts.
git statusinside thelibultrashipsubmodule was permanently dirty.Dropping the argument lets CMake place the output under the binary directory, which
is the normal arrangement.
This relocates libultraship's build output. Artifacts that previously appeared
inside the
libultraship/source tree now land under the build directory. Anyscripts, packaging steps, IDE configuration or CI tooling that reference build
artifacts by a path inside
libultraship/will need those paths updated. Nothingelse in the build graph changes — the target name, dependencies and link line are
all as before.
Testing
CMake configure was exercised to confirm the file still parses and that both
changed call sites are reached. No syntax or parse errors; the only failures were
unrelated missing submodule checkouts in the test tree.
Portions of this change were prepared with AI assistance; the diff, the reasoning
and the measurements above were reviewed and verified by hand.