Ideas for reduced compile times

[coderedart]

Replace ureq with curl

rusty_v8 is a project maintained by deno people similar to rust-skia.

1. huge complex c++ libraries wrapped with custom C bindings
2. deal with gn/ninja from chromium/google family
3. provide a static "linkable" library on github, to avoid compiling from source for 15 minutes.

They do try to use python first, but when it fails, rusty_v8 simply use a curl command in their build script to download their static binary. I think skia-safe could remove a build dependency by doing so which should improve compile times.

generate bindings once per release

rusty_v8 actually maintain a single bindings.cc file with custom bindings. And the rest of the safe wrapping just use these c functions.
skia-safe can do this too, by only generating bindings before publishing a release, and only committing the generated bindings.rs file in the sources. or even maintain manual bindings.

1. you avoid bindings crate as build dependency.
2. you can publish it to docs.rs :)

Ofcourse, we could actually go a step further and use skiasharp's C bindings which are maintained by hand.

NOTE: it is missing some bindings to modules like paragraph and they may not be on the latest skia yet.

[pragmatrix]

These are good ideas and I appreciate that you care. Here are some of my initial arguments, which may be wrong, outdated, biased, and therefore - of course - open for a more detailed discussion:

• Relying on an external curl command would rule out platforms on which curl is not installed (what about Windows?). Also ureq is required when the feature svg is enabled (we may switch to reqwest in the future).
  It's probably possible to create a new skia-bindings feature that replaces the use of ureq with a curl invocation, a PR would be great.
• The bindings file and some of the structures are platform dependent, this is because Skia uses a wide range of C++ includes. Not sure how this can be solved without breaking backward compatibility, specifically if C++ types can be used as transmutable Rust values. A first step would try to export only structs to Rust that are platform independent and adjust the wrappers for the rest.
• Using skia-sharp's C bindings would mean to rewrite all the Rust wrappers. And as you've noted, SkiaSharp was never good in keeping up to date with Skia in the past, which means to me that the only option is to take over maintenance of the C bindings, which may be a worthy attempt for another Skia binding project, but out of scope for rust-skia for the time being or as long I can't see a clear migration path.

[coderedart]

(what about Windows?)

windows 10+ has curl inbuilt since 2017 :) and for older platforms, we could a feature-gated ureq maybe.

Also ureq is required when the feature svg is enabled

can be replaced with curl too. with a fallback to feature-gated ureq (similar to above). network call latency will hide any performance loss of using curl (with cmdline) over ureq.

some of the structures are platform dependent

This is where skiasharp's bindings would be helpful. They simply use

1. opaque types for most cpp structs and
2. C structs for dumb c types (rect, irect, vector2 etc..) in a sktypes.h file

with a few ifdefs to help with platform specific quirks. OTOH, rust-skia using bindings generator would actually translate structs to rust types.

A good example is SkPaint. on my system, rust-skia would generate this

#[repr(C)]
pub struct SkPaint {
    pub fPathEffect: sk_sp<SkPathEffect>,
    pub fShader: sk_sp<SkShader>,
    pub fMaskFilter: sk_sp<SkMaskFilter>,
    pub fColorFilter: sk_sp<SkColorFilter>,
    pub fImageFilter: sk_sp<SkImageFilter>,
    pub fBlender: sk_sp<SkBlender>,
    pub fColor4f: SkColor4f,
    pub fWidth: SkScalar,
    pub fMiterLimit: SkScalar,
    pub __bindgen_anon_1: SkPaint__bindgen_ty_1, // bitfields
}

OTOH, skia-sharp just makes it an opaque type

typedef struct sk_paint_t sk_paint_t;

Obviously, rust-skia's version seemed better at face value because rust can inline/optimize them. But once i turn on link time optimization, i don't think there would be a performance difference.

OTOH, skia-sharp's design has a different advantage.

1. you can make a dll out of it. as long as the fn signatures remain the same, we just need to pull the new skia-dll without recompiling our own rust-app.
2. remove bindings generator from dependencies. regardless of static or dynamic linking. If you ever wanted to generate bindings (maybe to generate signatures for a new skia module like graphite), you can use the cli to do it manually, instead of adding it as a build-dependency.
3. allowing stable C API means, we don't have to parse C code at all and make this a pure rust crate. This is extremely useful for my usecase of using skia from wasm32-wasi where Cpp compilation is very brittle. I made a separate issue about that .

rusty_v8 does the same thing by generating bindings once and then, manually maintain them by hand. they just delcare the relevant extern "C" fns in the rust files, and don't even bother using bindgen to get them out of bindings.cc file.

I am very new to skia, so i have no idea how much work keeping up with latest skia would actually be. if someone needs the latest skia module's bindings, they can contribute them if we provide a simple guide. The current method of bindgen is actually more scary (personal opinion), as random transmute or handle thing could quickly lead to UB. communicating via C API is probably easier for most people as its just extern "C" and adding a matching entry in a header file.

[coderedart]

create a new skia-bindings feature that replaces the use of ureq with a curl invocation

I just removed ureq by default, which will use curl. and added a no-curl feature to pull in ureq. It removes 30-ish out of 110 dependencies and brings down compile times by 1-2 seconds. serde_json + bindgen still seem to be huge hitters.

[coderedart]

There are a few obvious crates that are easy to replace

name	purpose	replacement	reason
ureq	download binaries	curl binary from OS	avoids ureq dependency completely
serde_json	to parse vcs_info.json to get git repo hash	tinyjson	faster compiles than serde_json
toml	parse skia and depot_tools versions from Cargo.toml	manual parsing	remove dep. as Cargo.toml is our file, we don't need full toml parsing capabilities

Some measurements.

1. tinyjson and serde_json are equivalent in functionality. tinyjson just doesn't have any deps, so i replaced serde_json without any fallback.
2. with ureq and toml i made sure to keep them feature gated. ureq can be enabled with no-curl feature. toml can be enabled with use-toml feature.
3. I made an empty project called skia_temp that has my local rust-skia repo as dependency. disabled all features except binary-cache.
4. clean cargo + force binary download + release mode

To make the effect clear, i restricted the number of jobs to 1. Then, i made the measurement again with full parallel jobs (12 vcores on ryzen 2600x).

Curl and manual parsing

name	value
cmd	cargo clean && FORCE_SKIA_BINARIES_DOWNLOAD=1 cargo build -p skia_temp --release --timings --jobs 1
deps count	62
Max concurrency	1 (jobs=1 ncpu=12)
Total time	43.1s

ureq and toml

name	value
cmd	cargo clean && FORCE_SKIA_BINARIES_DOWNLOAD=1 cargo build -p skia_temp --release --timings --jobs 1 --features use-toml,no-curl
deps count	96
Max concurrency	1 (jobs=1 ncpu=12)
Total time	60.0s

Ofcourse, with more parallel compilation, the gains are much lower.

Curl and manual parsing

name	value
cmd	cargo clean && FORCE_SKIA_BINARIES_DOWNLOAD=1 cargo build -p skia_temp --release --timings --jobs 1
deps count	62
Max concurrency	12 (jobs=12 ncpu=12)
Total time	15.6s

ureq and toml

name	value
cmd	cargo clean && FORCE_SKIA_BINARIES_DOWNLOAD=1 cargo build -p skia_temp --release --timings --jobs 1 --features use-toml,no-curl
deps count	96
Max concurrency	12 (jobs=12 ncpu=12)
Total time	18.0s

the measurements are mostly consistent within +-1 second error margin. we get 2-3 seconds faster compiles with curl and manual parsing, and remove roughly 34 deps (eco-friendly i guess?).

I sent a PR, but you should revert the commit before testing because those skia binaries would not match the commit hash and start a full skia build. #900

[coderedart]

Okay. So, i am sort of wrong. while removing ureq and toml crates helped. The major culprit is still generating the full bindings to skia.

Here's the cargo timing html files cargo_timings.zip if you want to check them yourself.

The pipeline is basically bottlenecked by

name	individual time (seconds)	cumulative time by chaining (seconds)
proc-macro and quote	1s	1s
syn	4s	5s
prettyplease	1s	6s
bindgen	1.5s	7.5s
build skia-bindings build.rs	1.5s	9s
run skia-bindings build.rs to generate bindings	2s	11s
compile skia-bindings	1.5s	12.5s
compile skia-safe	3.5s	15s
total	0s	15s

When i disabled prettyplease feature of bindgen, it actually too longer (around 35 seconds) to finish. so, we can't really eliminate any of the dependencies in this chain to make it go faster.

There are some other crates like regex or flate2 which can be removed too to reduce deps count. But there is just no way to bring the compile times below 15-ish seconds when we generate bindings on every single clean build.

I will let you decide how to proceed further. If you do decide to go with the manual bindings (either skiasharp or otherwise), i can help do the manual monkey work like changing the existing safe wrappers with new bindings etc..

[pragmatrix]

Thanks.

I agree that removing ureq and even make the curl invocation seems to be a great way to reduce the number of dependencies and build times. Therefore, I suggest to remove the feature gate in #900 and take some risk. But removing serde_json and toml I see as micro optimizations with tradeoffs. For serde_json introducing another crate that is most likely used by downstream crates is probably not worth the effort (maintainers for downstream crates may step in here), and for toml, maintaining a hand written extractor, may not be worth the maintenance effort and may not be compatible with future updates of the file it parses. Both of these changes are short term wins and may cause long term problems. I would like to postpone them until we settle on the bindings, because this is the real deal in terms of build time optimizations and docs.rs generation. And yes, the idea of rethinking the bindings is growing on me.

Regarding updates. rust-skia is meant to provide up to date Skia versions, which - as long I am maintainer - is guaranteed to the best of my abilities and an important feature of rust-skia, meaning that rust-skia needs to maintain its own set of bindings.

Regarding the bindings. I would like to do this based on what's already there. Therefore, I suggest a (perhaps) long term effort to transform all the C bindings in this project so that we can use one manual invocation of bindgen that results in one Rust bindings file that is compatible with all platforms. Failing size, alignment, and offset checks that bindgen generates can be used as an indicator for which types need to be made opaque.

[coderedart]

I will be reverting serde_json and toml changes. But i hope if we change the bindings, we will skip them completely to avoid hacks like reading data from cargo manifest or vcs_info.json. Because combined, they will easily take more than 5 seconds. but both are used only once.

PR at #901

[coderedart]

Therefore, I suggest a (perhaps) long term effort to transform all the C bindings in this project s

I kinda missed this smh. Anyway, would it not be easier to start from scratch with a cleaner approach? Here's the rough idea i had in mind.

1. fork skia repo as skia-sys (just like glfw-sys). or submodule it in a new repo. but separate from this main repo.
2. copy the include/c and src/c folders from skia-sharp's repo. avoid most work with manual "C" bindings. copy the rest from our skia-bindings crate if necessary or write new.
3. add build_tools as a submodule.
4. run bindgen_cli to generate rust from the include/c headers.
5. remove all the clutter except for the actual useful stuff.
6. rename it to lib.rs
7. create a feature called build_src and check in build.rs file to only build it if that feature is enabled and if we an env var "DOCS_RS" is not set (to build on docs.rs). and the rest of the build.rs stuff
8. publish a release. CI will build a static (and even a dynamic) lib with all features enabled per platform. (as long as they are additive, is there a problem with this?).
9. The crate will be published with skia source files and even buid_tools (by using cargo's include section to whitelist required files). crates.io (and docs.rs) will actually grant exceptions for really huge crates. Raise the crate size limit? rust-lang/crates.io#195 is the first request and it also happens to be for skia (from servo folks) :)

The merits of this approach are:

1. you separate the messy c++ part into a separate repo.
   • This allows all the build tests in workflows/platforms, generating static/dynamic libs and other mess to be dealt with only on that skia-sys repo.
   • we can keep this repo clean with skia-safe and examples, which will be easier to contribute to. eg: docs (god knows skia needs more docs).
2. updating skia is just pulling upstream changes or bumping submodule (of skia and build_tools) to a new commit.
   • we can probably write a github workflow that uses bindgen_cli to check that our rust definitions match the bindgen generated defs, and builds static/dynamic libs.
3. uploading src to crates.io allows us to skip a lot of complexity that we have with current way of metadata approach where we pull sources from various repos with specific commit hashes. This will allow some more things.
   • EDIT: ignore the below points. i completely forgot that we still need to run git-sync-deps. but the above advantage of avoiding cargo metadata to pull skia/depot_tools still applies.
   • "vendor" skia crate. https://doc.rust-lang.org/cargo/commands/cargo-vendor.html
   • continue using when offline due to accidentally running "cargo clean"
   • firewall. nobody likes a crate that connects to internet. hosts/sites like docs.rs, or some other CI workflow docker machine of some downstream crate.
   • github being down temporarily should not lead to a problem.
   • build.rs can be simplified. you either curl the libskia.a or you do a full build. The only thing build.rs should focus on is things like using env vars for defining gn or ninja args etc.. or detecting features to enable modules. no need for serde_json, toml, flate2, tar etc..

As i mentioned already, If you take care of the higher level decisions and build system stuff, i can contribute manual labor like porting over skia-safe's usage of extern functions.

[pragmatrix]

Anyway, would it not be easier to start from scratch with a cleaner approach?

Perhaps, but this is another project, with a new set of problems. May be a disrupting one for rust-skia. If so, that is fine, but I have to stay loyal to what's already there. If I wouldn't, the intricacies of the Rust API that has been drafted and refined over the years would be lost and users of rust-skia would be forced on a completely different API surface and a lot of porting effort. Getting parity will involve a huge effort, and I don't see an actual added value besides faster build times and reduced adoption resistance. These are important factors, but not the ones I am focusing on.

I have evaluated a few options to head to the suggested path from the state this project is already at, and this seems to be as very hard, nearly impossible. But I will keep your ideas in mind and continue heading to what you suggested and similar projects do already. If you want to continue with rust-skia, try to think more in these terms, or otherwise set up a new binding project, which I am happy to promote if it turns out to be an viable alternative, because long term, and from my perspective, managing "build system stuff" is the easiest part, because this is actually where contributors are seem to be stepping in quite frequently.

[coderedart]

I am working on ckia based on skiasharp's c bindings. It seems like skia with fast compile times is doable (with zero build dependencies).

I thought i will mention it here, in case you can see any obvious mistakes that i am making.

1. fork skia at https://github.com/coderedart/skia . Its README explains the changes, but basically
   1. add a ckia folder with c bindings headers/sources at project root.
   2. edit BUILD.gn to add the new "C" headers and sources to skia's component configuration.
   3. add .github/workflows to make sure that the bindings are working on different platforms.
   4. When a new milestone releases, just copy the ckia and .github dirs to a new branch based on latest milestone. edit BUILD.gn. and that's it. Fix any errors which the CI will catch (or build locally). maybe create a new release with a source tarball?.
2. create a ckia_sys project at https://github.com/coderedart/ckia_sys . This is where all the skia "build" stuff will stay. Its README should also explains its steps, but basically
   1. maintain a build.rs that is a mix of rust-skia + rusty_v8.
   2. The build.rs serves the following purposes:
      1. download pre-built libs using system curl and tar. maybe cache them in some user cache dir. eg: CSIDL_INTERNET_CACHE on windows and XDG_CACHE_HOME on linux etc..
      2. build from src (more or less a copy of rust-skia's skia-bindings crate) using input from env vars and features. we will pull the source from the skia fork from step 1. or use a local copy if env var is set.
      3. copy the built libs to some external folder (useful for dynamic/shared libraries or icudtl.dat).
      4. add the needed linker arguments.
   3. A workflow to tag a release with pre-built artifacts.
   4. Finally, manually run bindgen before every release to generate bindings.rs and paste them into lib.rs.
3. create a ckia project at https://github.com/coderedart/ckia . This is similar to skia-safe. We wrap all the unsafe functions in Safe code.

The result is a crate that builds within 4 seconds. we are bottlenecked on curl's network speed. Once we start caching the downloaded artifacts in a global place, it should probably be <1sec for building as we just print the link args and nothing else.

TBH, i kinda regret doing this. Getting skia to build was not easy, and my lack of c++ knowledge didn't help. If i would have spent this time contributing to a rust project like femtovg, it would have been atleast fun learning all kinds of vector graphics knowledge. Instead, i spent my time learning weird build system (which was going to be deprecated with bazel anyway) and c++.

But, i do love the compile times, being able to dynamically link skia -> skip building it, add skia bindings to wasm plugins etc..

Anyway, i will spend tonight finishing up the build.rs work by copying as much of rust-skia as possible. Just wanted to ask if you there's any specific platforms/weirdness i need to be worried about. or if this path will lead me to some kind of bad ending.

PS:

1. python tools/git-sync-deps already downloads a gn binary into the bin directory, based on current platform and python ./bin/fetch-ninja will download ninja binary into third_party/ninja/ninja. So, i was able to completely ignore the depot_tools repository. I was wondering if rust-skia should do that too? It may not be a huge improvement, but you get to remove one more dependency. could even make ninja a build time dependency just like python is.
2. tar is available on windows/mac (just like curl). so, maybe you can skip the flate2/tar build-deps. again, the bottleneck will still probably be bindgen, but you get to remove more dependencies in favor of system provided utils.