aclysma / profiling

Provides a very thin abstraction over instrumented profiling crates like puffin, optick, tracy, and superluminal-perf.
Apache License 2.0
319 stars 39 forks source link

Guidance for build dependencies #73

Open attackgoat opened 3 months ago

attackgoat commented 3 months ago

When [build-dependencies] use profiling it can cause trouble building the target crate. If possible I'm looking for advice and updates that could be made to the documentation.

Background

Recently the image crate was updated to include a dependency on rav1e (via ravif). This dependency uses profiling in the normal way, but fails when image is used from build script of a profiling build. This is somewhat complex so I made a simple repro case.

Sample repository #### Problem repo https://github.com/attackgoat/profiling-test/tree/main - `my-bin` includes `foo-lib` which uses `child-lib` in a build script - `child_lib` uses `profiling` - `my-bin` includes `bar-lib` which uses `profiling` #### How to reproduce the issue ```bash cargo run cargo run --features profile-with-puffin ``` The second `run` generates: ```bash error[E0433]: failed to resolve: could not find `puffin` in `profiling` --> /child-lib/src/lib.rs:1:1 | 1 | #[profiling::function] | ^^^^^^^^^^^^^^^^^^^^^^ could not find `puffin` in `profiling` | = note: this error originates in the attribute macro `profiling::function` ``` #### How to fix: https://github.com/attackgoat/profiling-test/compare/main...attackgoat:fix

Proposed fix

If I understand what is happening here correctly, the procmacro crate is configured to use puffin, but the profiling crate used by the build is not similarly configured. It's unclear to me if this is a Cargo bug or it is intended that procedural macros are only built once and not separately for the build dependencies.

The fix I used in the sample repository is to expose profile-with-* features in any crate whose dependencies expose such features, and enable them in a pass-through manner. foo-lib does not use profiling but provides the features so that my-bin can enable profiling in child-lib.

This works, in that I can now use the image crate in build scripts and also use profiling on the resulting binary, but the downside is that I'll have to go ask authors of these "middle" crates to expose and maintain these features in their creates too. I also cannot make a build which does not profile rav1e but does profile the code I'm interested in.

Is there a better way to do this?

aclysma commented 3 months ago

Your approach to have crates pass features along to sub crates is what I've been doing in my own multi-crate projects. I don't know of a better way at the moment. One idea comes to mind - can you use cargo tree and confirm that you just have a single version of the profiling and profiling-procmacro crates? I'm wondering if it's possible that the feature is enabled for version 1.0.x but not for 1.0.y.

Another workaround would be to have a hacked version of the profiling crate that hardcodes a particular backend as always on and then use cargo's crate patching functionality to force using your locally modified version of the profiling crates. This avoids any reliance on features to accomplish the task at hand. It's ugly and I don't love it, but I bet it would work. Although this still means you're profiling all crates, not just a specific crate.

attackgoat commented 3 months ago

can you use cargo tree and confirm that you just have a single version of the profiling and profiling-procmacro crates?

Yes - in the actual case of image+rav1e with the crate I'm intending to profile, they're all the same version inspected with cargo tree -e features. The sample repository I created also demonstrates this by locating all crates in one easy to debug place.

I've found a solution I'm happy with (see the commits on this branch of the sample repository). This keeps the documented public API of profiling unchanged and adds stub implementations for each profiler when the feature is not enabled. The proc macro crate then does not require any features and calls all profilers, letting the compiler remove the no-op ones.

This allows you to profile only the crate you're interested in, example - bar-lib is profiled here with puffin while child-lib is not:

cargo tree -e features --features profile-with-puffin

my-bin v0.1.0 (/profiling-test/my-bin)
├── bar-lib feature "default"
│   └── bar-lib v0.1.0 (/profiling-test/bar-lib)
│       └── profiling feature "default"
│           ├── profiling v0.1.0 (/profiling-test/profiling)
│           │   ├── profiling-procmacros feature "default"
│           │   │   └── profiling-procmacros v0.1.0 (proc-macro) (/profiling-test/profiling-procmacros)
│           │   │       ├── quote v1.0.35
│           │   │       └── syn feature "full"
│           │   │           └── syn v2.0.55 (*)
│           │   └── puffin feature "default"
│           │       └──  [...REMOVED FOR CLARITY...]
│           └── profiling feature "procmacros"
│               ├── profiling v0.1.0 (/profiling-test/profiling) (*)
│               └── profiling feature "profiling-procmacros"
│                   └── profiling v0.1.0 (/profiling-test/profiling) (*)
└── foo-lib feature "default"
    └── foo-lib v0.1.0 (/profiling-test/foo-lib)
        [build-dependencies]
        └── child-lib feature "default"
            └── child-lib v0.1.0 (/profiling-test/child-lib)
                └── profiling feature "default"
                    ├── profiling v0.1.0 (/profiling-test/profiling)
                    │   └── profiling-procmacros feature "default" (*)
                    └── profiling feature "procmacros"
                        ├── profiling v0.1.0 (/profiling-test/profiling) (*)
                        └── profiling feature "profiling-procmacros"
                            └── profiling v0.1.0 (/profiling-test/profiling) (*)

In the above example child-lib is a build dependency of foo-lib and exposes profiling features but does not have any enabled. This fixes the compile issue I hit. This does not somehow enable multiple-profilers at once, and doing so continues to fail to compile.

aclysma commented 3 months ago

I synced the project and found a pretty straightforward solution. Add this to any of the crates you control, such as my-bin:

[build-dependencies]
profiling = "1.0"

And enable the profile-with-puffin feature on it. You can have profiling listed as both a dependency and a build-dependency simultaneously. This appears to propagate the feature as needed.

Does this work for you?

attackgoat commented 3 months ago

Yes that works perfectly and fixes my issue with image + rav1e also!

This is better than either fix I suggested and seems like a good item to document for cases where build dependencies use profiling.