libexpr: Add EvalProfiler and use it for FunctionCallTrace#13219
Merged
Mic92 merged 2 commits intoNixOS:masterfrom May 18, 2025
Merged
libexpr: Add EvalProfiler and use it for FunctionCallTrace#13219Mic92 merged 2 commits intoNixOS:masterfrom
EvalProfiler and use it for FunctionCallTrace#13219Mic92 merged 2 commits intoNixOS:masterfrom
Conversation
xokdvium
commented
May 16, 2025
| auto duration = std::chrono::high_resolution_clock::now().time_since_epoch(); | ||
| auto ns = std::chrono::duration_cast<std::chrono::nanoseconds>(duration); | ||
| printMsg(lvlInfo, "function-trace entered %1% at %2%", pos, ns.count()); | ||
| printMsg(lvlInfo, "function-trace entered %1% at %2%", state.positions[pos], ns.count()); |
Contributor
Author
There was a problem hiding this comment.
Since #13211 operator[] is much cheaper to call, since PosTable now caches the line information.
xokdvium
added a commit
to xokdvium/nix
that referenced
this pull request
May 17, 2025
Rough of an initial draft implementation based on the `EvalProfiler` interface. The interface/options is still TBD, so this serves as an technical POC. This depends on NixOS#13219. Co-authored-by: Jörg Thalheim <joerg@thalheim.io>
This patch adds an EvalProfiler and MultiEvalProfiler that can be used to insert hooks into the evaluation for the purposes of function tracing (what function-trace currently does) or for flamegraph/tracy profilers. See the following commits for how this is supposed to be integrated into the evaluator and performance considerations.
This wires up the {pre,post}FunctionCallHook machinery
in EvalState::callFunction and migrates FunctionCallTrace
to use the new EvalProfiler mechanisms for tracing.
Note that branches when the hook gets called are marked with [[unlikely]]
as a hint to the compiler that this is not a hot path. For non-tracing
evaluation this should be a 100% predictable branch, so the performance
cost is nonexistent.
Some measurements to prove support this point:
```
nix build .#nix-cli
nix build github:nixos/nix/d692729759e4e370361cc5105fbeb0e33137ca9e#nix-cli --out-link before
```
(Before)
```
$ taskset -c 2,3 hyperfine "GC_INITIAL_HEAP_SIZE=16g before/bin/nix eval nixpkgs#gnome --no-eval-cache" --warmup 4
Benchmark 1: GC_INITIAL_HEAP_SIZE=16g before/bin/nix eval nixpkgs#gnome --no-eval-cache
Time (mean ± σ): 2.517 s ± 0.032 s [User: 1.464 s, System: 0.476 s]
Range (min … max): 2.464 s … 2.557 s 10 runs
```
(After)
```
$ taskset -c 2,3 hyperfine "GC_INITIAL_HEAP_SIZE=16g result/bin/nix eval nixpkgs#gnome --no-eval-cache" --warmup 4
Benchmark 1: GC_INITIAL_HEAP_SIZE=16g result/bin/nix eval nixpkgs#gnome --no-eval-cache
Time (mean ± σ): 2.499 s ± 0.022 s [User: 1.448 s, System: 0.478 s]
Range (min … max): 2.472 s … 2.537 s 10 runs
```
Mic92
reviewed
May 18, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Having and extensible framework for evaluation profiling is much needed if we want to get flamegraph/tracy-based profilers #9967, #11373. This patch adds a
EvalProfilerinterface andMultiEvalProfiler, which supportpreFunctionCallHookandpostFunctionCallHookthat get call on entering and exitingcallFunction. The actual hook invocation is guarded behind a simple cached boolean check with an[[unlikely]]annotation. This machinery is enough to reimplement existingFunctionCallTrace.I hope this patch can pave the way forward for #9967, #11373 in an extensible and well-architected manner. Since the actual profiler implementation is hidden behind an interface, multiple profilers can be easily supported, so flamegraph/tracy can hopefully co-exist behind a
--eval-profilerflag. (Current settingtrace-function-callscan also be absorbed into that hypothetical feature).(Inlined message from the second commit, which has non-tracing performance measurements)
Note that branches when the hook gets called are marked with
[[unlikely]]as a hint to the compiler that this is not a hot path. For non-tracing
evaluation this should be a 100% predictable branch, so the performance
cost is nonexistent.
Some measurements to support this point:
(Before)
(After)
Context
#9967
#11373
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.