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

try to run generate in parallel #2247

Open
dev-ardi opened this issue Sep 26, 2024 · 14 comments
Open

try to run generate in parallel #2247

dev-ardi opened this issue Sep 26, 2024 · 14 comments

Comments

@dev-ardi
Copy link

dev-ardi commented Sep 26, 2024

There are lots of oportunities to parallelize the generating code using something like rayon and it will speed up some builds by a lot!

@badboy
Copy link
Member

badboy commented Sep 26, 2024

I'm not against introducing parallelization where it makes sense, but it does add complexity.
I don't think the code generation itself is much of a bottleneck right now. So before we jump onto introducing more complexity in how it's done I'd like to see some measurements that it is indeed "a lot".

@dev-ardi
Copy link
Author

In my project it takes 4m, it is a bottleneck for my workflow.

@badboy
Copy link
Member

badboy commented Sep 26, 2024

In my project it takes 4m, it is a bottleneck for my workflow.

Honestly that is surprising to me.
Have you looked closer what exactly is slow there?
How big is your API? What languages are you generating it for?

@dev-ardi
Copy link
Author

I haven't profiled it, no. I'm generating only for kotlin,

The generated file is 675KB

@badboy
Copy link
Member

badboy commented Sep 26, 2024

That's honestly surprising it takes that long and I think there might be other things going.
If this is for a single language I don't see how we can parallelize that with the current architecture.

To note for Glean it's near-instantenous:

❯ time cargo -q uniffi-bindgen generate --no-format glean-core/src/glean.udl --out-dir out --language kotlin
cargo -q uniffi-bindgen generate --no-format glean-core/src/glean.udl  out    0.09s user 0.03s system 31% cpu 0.371 total

resulting in a 424KB Kotlin file.


Do those 4 minutes include fully building uniffi-bindgen first?
Are you using UDL or the macro approach?

@dev-ardi
Copy link
Author

dev-ardi commented Sep 26, 2024

We're using the macro approach, and I'm not including building uniff-bindgen. I'm also building it as release.

@badboy
Copy link
Member

badboy commented Sep 26, 2024

Is your code open-source so we can take a look at that?

@dev-ardi
Copy link
Author

It's unfortunately not...

@mhammond
Copy link
Member

mhammond commented Oct 4, 2024

If time cargo -q uniffi-bindgen generate... takes 4m, it might somehow be the symbol extraction from the library path you are passing, done by goblin?

@dev-ardi
Copy link
Author

dev-ardi commented Oct 4, 2024

how can I know what's taking long in the build? Do you have any form of self profiling?

@mhammond
Copy link
Member

mhammond commented Oct 4, 2024

We don't have any profiling but we've also never seen this take more than a trivial amount of time. You probably need to use a local build with manually added println statements.

What command exactly are you executing?

@dev-ardi
Copy link
Author

dev-ardi commented Oct 28, 2024

cargo run \
        --release \
        --package uniffi-bindgen generate \
        --library target/x86_64-linux-android/debug/[my_library].so \
        --language kotlin \
        --config  uniffi/uniffi.toml \
        --out-dir uniffi/android/lib/src/main/java

The measured time of 4m was done after building this.

@oguzkocer
Copy link

We were having a similar issue with long build times for Kotlin bindings generation and it turns out ktlint was the cause of it. Passing --no-format decreased the build time from ~10 minutes to under a second.

It'd probably be worth adding a log output for when the binding generation is complete and maybe even mention that the build will now start formatting the code. That way if anyone else has a similar issue, they can quickly identify the cause of it.

@dev-ardi
Copy link
Author

This is it!
I don't think that having klint run by default is a good thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants