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

Prebuilt/caching libraries for BRN #1270

Open
CedricGuillemet opened this issue Jul 27, 2023 · 12 comments
Open

Prebuilt/caching libraries for BRN #1270

CedricGuillemet opened this issue Jul 27, 2023 · 12 comments
Assignees
Labels
enhancement New feature or request infrastructure
Milestone

Comments

@CedricGuillemet
Copy link
Contributor

CedricGuillemet commented Jul 27, 2023

In order to speed up BRN builds, I want to evaluate the feasibility of having (some) prebuilt libraries of BabylonNative.
As discussed with @bghgary , because of JS engine, it won't be possible to prebuild everything. Does it make sense to have more clear separation for some libraries (JS engine layer + internal functionnality).
Note: I'm also wondering if it makes sense to not build everything for BN local builds or CI.

List of libraries dependent of JSEngine:

  • NativeXR
  • NativeCamera
  • NativeEngine
  • napi
  • Console
  • JSRuntime
  • NativeOptimizations
  • v8inspector
  • ScriptLoader
  • XMLHttpRequest
  • AppRuntime
  • Graphics
  • Window
  • Canvas
  • AbortController
  • Externaltexture
  • NativeTracing

Independent ones:

  • edtaa3
  • etc1
  • tinyexr
  • pvrtc
  • OGLCompiler
  • OSDependent
  • GenericCodeGen
  • llhttp(?)
  • squish
  • iqa
  • arcana
  • bx
  • etc2
  • astc-encoder
  • nvtt
  • spirv-cross-glsl
  • urllib
  • bimg
  • MachineIndependent
  • glslang
  • openxr_loader
  • bgfx
  • SPIRV
  • xr
  • URL(?)
  • WebSocket(?)

Excepted ABI:
win32 : x86 Debug + Release, win_x64 Debug + Release
arm64, arm7(?), x86, x86_64
ios: arm64, x86_64, arme(?)

@ryantrem
Copy link
Member

Would this specifically be local builds, build agents, or both?

@CedricGuillemet
Copy link
Contributor Author

both. Maybe some option in cmake to download binaries or build them.
I'm not sure this will be faster on our local PC because they are way faster than CI VMs.

@ryantrem
Copy link
Member

If CI VMs are the main concern, could we use GitHub workflow caching for the build output? This would effectively make the builds incremental. I’m not sure how hard it wold be to generate a correct cache key for build outputs though. https://docs.github.com/en/actions/using-workflows/caching-dependencies-to-speed-up-workflows#

@CedricGuillemet
Copy link
Contributor Author

CedricGuillemet commented Jul 27, 2023

Yes, it makes sense to save some binaries when a PR is merged, using BN commit hash as a key. And reuse with all builds following as long as it's the same BN commit. BN is not updated very often so it makes total sense.

@CedricGuillemet
Copy link
Contributor Author

I've used MSVC to get local build times. I'm keeping here libraries with >2seconds times.

squish 6
iqa 6
llhttp 8.7
spirv-cross-core 12
arcana 10
bx 13
astc-encoder 24
nvtt 11.7
spirv-cross-glsl 16.4
url-lib 18.8
spirv-cross-hlsl 7
bimg 11
openxr_loader 40
MachineIndependent 37
bgfx 11.3
xr 9
SPIRV 11.4

253.3 seconds

v8inspector 25
XMLHttpRequest 6.5
ScriptLoader 6.8
AppRuntime 6.6
Graphics 13.4
NativeXr 6.2
NativeCamera 8.2
Canvas 13.6
NativeEngine 19.4
native-input 10.5

116.2 seconds

so ~70% of build time can be cached and reused

@ryantrem
Copy link
Member

If it’s just build caching, then I’d think the binaries from both your lists could be included as well.

@CedricGuillemet
Copy link
Contributor Author

We discussed that with @bghgary and because of changes in JS interface, react,... this might not be possible.

@ryantrem
Copy link
Member

Since this would be caching specifically for the BN build within BRN, then BN is always configured with a specific JS engine etc., so I would expect caching to be fine on all binaries. Not sure what you and @bghgary discussed though so maybe I’m missing something.

@CedricGuillemet CedricGuillemet changed the title Prebuilt libraries for BRN Prebuilt/caching libraries for BRN Jul 27, 2023
@bghgary bghgary added this to the 7.0 milestone Jul 31, 2023
@bghgary bghgary added enhancement New feature or request infrastructure labels Jul 31, 2023
@CedricGuillemet
Copy link
Contributor Author

CedricGuillemet commented Sep 11, 2023

I did some tests today with Github Caching and got these issues:

  • Total cache per repo is limited to 10Gb (might be good for BN for not BRN with 3 platforms)
  • cache untouched for 7 days will be removed
  • CMake does changes outside the build directory that can't be cached (node install for example)
  • because source code files are more recent after checkout, a rebuild is performed anyway

@CedricGuillemet
Copy link
Contributor Author

CedricGuillemet commented Sep 18, 2023

I tried to reuse, locally, bgfx static libraries.
First, I build the PG as usual, then, I change the cmake file in dependencies to reuse the library and headers instead of adding bgfx.cmake subdirectory.

set(BGFX_DIR "${CMAKE_CURRENT_SOURCE_DIR}/bgfx.cmake/bgfx" CACHE STRING "Location of bgfx." )
set(BIMG_DIR "${CMAKE_CURRENT_SOURCE_DIR}/bgfx.cmake/bimg" CACHE STRING "Location of bimg." )
set(BX_DIR "${CMAKE_CURRENT_SOURCE_DIR}/bgfx.cmake/bx" CACHE STRING "Location of bx." )

add_library(bgfx STATIC IMPORTED)
set_property(TARGET bgfx PROPERTY IMPORTED_LOCATION "${CMAKE_CURRENT_BINARY_DIR}/Dependencies/bgfx.cmake/Debug/bgfx.a")
target_include_directories(bgfx INTERFACE "${BGFX_DIR}/include")

But it didn't work and include is not added to the build command line
and I end up seeing this error:

BgfxCallback.h:6:10: fatal error: 'bgfx/bgfx.h' file not found
#include <bgfx/bgfx.h>

Any idea why it happens ?

EDIT: Adding GLOBAL fixes the issue. But imported targets cannot be installed.

@CedricGuillemet
Copy link
Contributor Author

WIP using prebuilt on win32. bgfx, glslang and spirv-cross
before:
image
after:
image

@CedricGuillemet
Copy link
Contributor Author

CedricGuillemet commented Oct 11, 2023

I'm reaching a point where cmake needs to run to fetch content (jsruntimehost, arcana,...) while also needing the prebuilt cache so it can import the libraries. But the cache (in order to be invalidated) needs a commit hash. A way to do it is to run cmake 2 times:

  • first to fetch content
  • second to import library or create build projects for 3rd party libs.

That's not something I'd like to do because of less maintainable cmake scripts and the benefits will be low (around 3 minutes per build)
The problem is more BRN builds duration than BN ones.

@thomlucc thomlucc modified the milestones: 7.0, Future Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request infrastructure
Projects
None yet
Development

No branches or pull requests

4 participants