Skip to content

libexpr: plug ExprCall memory leak#14623

Merged
Ericson2314 merged 3 commits into
NixOS:masterfrom
Radvendii:exprcall-alloc-shvach
Nov 23, 2025
Merged

libexpr: plug ExprCall memory leak#14623
Ericson2314 merged 3 commits into
NixOS:masterfrom
Radvendii:exprcall-alloc-shvach

Conversation

@Radvendii
Copy link
Copy Markdown
Contributor

This PR does a similar transformation as #14539, but to ExprCall rather than ExprAttrs. It moves the storage into the arena after parsing by using std::optional<std::pmr::vector<Expr *>> instead of std::vector<Expr *>.

I verified that this does plug the memory leak using ASAN.

Unlike #14539, which slowed parsing down a bit, this seems to come with a tiny speed up. Not sure why it's affecting speed differently.

Performance numbers
$ poop -d 30000 '../master/result/bin/nix-instantiate --parse ../../nixpkgs/pkgs/development/haskell-modules/hackage-packages.nix' '../feature/result/bin/nix-instantiate --parse ../../nixpkgs/pkgs/development/haskell-modules/hackage-packages.nix'
Benchmark 1 (85 runs): ../master/result/bin/nix-instantiate --parse ../../nixpkgs/pkgs/development/haskell-modules/hackage-packages.nix
  measurement          mean ± σ            min … max           outliers         delta
  wall_time           353ms ± 3.82ms     347ms …  370ms          3 ( 4%)        0%
  peak_rss            108MB ±  215KB     108MB …  109MB          0 ( 0%)        0%
  cpu_cycles         1.31G  ± 14.4M     1.29G  … 1.39G           3 ( 4%)        0%
  instructions       3.70G  ± 8.76K     3.70G  … 3.70G           1 ( 1%)        0%
  cache_references   29.1M  ± 1.05M     28.4M  … 35.0M          12 (14%)        0%
  cache_misses       2.10M  ± 12.5K     2.07M  … 2.14M           3 ( 4%)        0%
  branch_misses      3.94M  ± 26.3K     3.91M  … 4.06M           3 ( 4%)        0%
Benchmark 2 (87 runs): ../feature/result/bin/nix-instantiate --parse ../../nixpkgs/pkgs/development/haskell-modules/hackage-packages.nix
  measurement          mean ± σ            min … max           outliers         delta
  wall_time           345ms ± 8.08ms     336ms …  398ms          9 (10%)        ⚡-  2.2% ±  0.5%
  peak_rss            109MB ±  212KB     108MB …  109MB          0 ( 0%)          +  0.9% ±  0.1%
  cpu_cycles         1.28G  ± 35.1M     1.24G  … 1.52G           9 (10%)        ⚡-  2.5% ±  0.6%
  instructions       3.68G  ± 8.80K     3.68G  … 3.68G           0 ( 0%)          -  0.5% ±  0.0%
  cache_references   29.9M  ± 3.41M     28.5M  … 57.6M           9 (10%)          +  2.6% ±  2.6%
  cache_misses       2.10M  ± 25.8K     2.06M  … 2.22M           6 ( 7%)          +  0.1% ±  0.3%
  branch_misses      4.02M  ±  102K     3.96M  … 4.54M           8 ( 9%)        💩+  2.2% ±  0.6%

Motivation

Plugging memory leaks will enable us to fuzz the parser. See the tracking issue for big-picture motivation.

Context


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@Radvendii Radvendii requested a review from edolstra as a code owner November 22, 2025 23:15
@Radvendii Radvendii mentioned this pull request Nov 22, 2025
42 tasks
@Ericson2314 Ericson2314 added this pull request to the merge queue Nov 23, 2025
Merged via the queue into NixOS:master with commit bd11043 Nov 23, 2025
16 checks passed
@edolstra edolstra mentioned this pull request Dec 9, 2025
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

Successfully merging this pull request may close these issues.

2 participants