firecrawl / firecrawl/pdf-inspector

Enable Link-Time Optimization (LTO) and codegen-units = 1 for Release builds (and possibly other options)

Open Beginner friendly
#95 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
19.1k
Forks
1.3k
Avg merge
9h 21m
Merged PRs (30d)
51

Description

Hi!

I noticed that in the `Cargo.toml` file [Link-Time Optimization](https://nnethercote.github.io/perf-book/build-configuration.html#link-time-optimization) (LTO) for the project is not enabled. I suggest switching it on since it will reduce the binary size (always a good thing to have) and will likely improve CPU performance a bit due to more aggressive compiler optimizations. Especially since you already [enabled](https://github.com/firecrawl/pdf-inspector/blob/8b63ceb084f75dfd4425129d1d90d5bca45054e4/napi/Cargo.toml#L17) for the Napi part. Additionally, `codegen-units = 1` (CU1) option can help too in a similar to LTO way, so I recommend to enable it as well - including the Napi part too.

I recommend enabling LTO only for Release builds so developers experience won't be affected by the increased build time. Actually, I can propose to use flags directly from this `ripgrep` [profile](https://github.com/BurntSushi/ripgrep/blob/cbc598f245f3c157a872b69102653e2e349b6d92/Cargo.toml#L76) (like stripping and other potentially useful for the project switches).

Basically, it can be enabled with the following lines to the root Cargo.toml file:

```
[profile.release]
codegen-units = 1
lto = true # FatLTO - the most aggressive LTO version

```

I have made quick tests (AMD Ryzen 9 5900x, Fedora 44, Rust 1.95, `cargo build --release` and `maturin build --release` build commands) - here are the results:

| Binary name \ Build Profile | Current Release | Release + FatLTO + CU1 |
| --- | --- | --- |
| `pdf2md` | 6.3 Mib | 5.0 Mib |
| `detect-pdf` | 6.3 Mib | 5.0 Mib |
| `libpdf_inspector.so` | 6.5 Mib | 5.1 Mib |
| Python Wheel | 2.4 Mib | 2.1 Mib |

Clean build time (`cargo build --release`):

* Current Release: 13s
* Release + FatLTO + CU1: 57s

Clean build time (`maturin build --release`):

* Current Release: 11s
* Release + FatLTO + CU1: 47s

Build time increase shouldn't be a problem since we enable it only for the Release profile - in this case, we would not affect the development lifecycle. If the proposed settings are added to the Cargo.toml file, all binaries will be built with the new options automatically.

Thank you.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start by reading the root Cargo.toml and napi/Cargo.toml profile configuration, then run cargo build --release and maturin build --release to establish the current behavior. Review the proposed release settings and any additional options before deciding their scope. Done means the release binaries and Python wheel build successfully with the agreed profile configuration and the size/build-time tradeoff is documented.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
build-system, performance
Issue type
Feature
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.