Skip to content

Merge environments of nested functions#3718

Draft
edolstra wants to merge 2 commits intomasterfrom
eval-optimisations
Draft

Merge environments of nested functions#3718
edolstra wants to merge 2 commits intomasterfrom
eval-optimisations

Conversation

@edolstra
Copy link
Member

Functions can now take more than one argument and function applications can apply more than one argument at a time. For non-partially applied functions/primops, this avoids allocation of application thunks. Also, it allows merging of environments (e.g. in a function like x: y: ..., we only allocate one environment).

On nix-env -qa -f '<nixpkgs>', this gives a ~8.5% speedup, reduces the number of environment allocations by 29.5% and the number of value allocations by 4.8%.

@edolstra edolstra marked this pull request as draft June 18, 2020 16:20
@andir
Copy link
Member

andir commented Nov 26, 2020

This looks like an amazing improvement. Is there something that could be done to get this merged?

@edolstra
Copy link
Member Author

IIRC, there was some segfault. I can have a look at cherry-picking at least the commits that don't break anything.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/tweag-nix-dev-update-5/10560/1

@stale
Copy link

stale bot commented Jun 16, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the stale label Jun 16, 2021
@stale stale bot removed the stale label Nov 4, 2021
@edolstra edolstra mentioned this pull request Nov 4, 2021
Previously an expression like 'x: y: ...' would create two
environments with one value. Now it creates one environment with two
values. This reduces the number of allocations and the distance in the
environment chain that variable lookups need to traverse.

On

  $ nix-instantiate --dry-run '<nixpkgs/nixos/release-combined.nix>' -A nixos.tests.simple.x86_64-linux

this gives a ~30% reduction in the number of Env allocations.
@andir
Copy link
Member

andir commented Nov 7, 2021

5253cb4 seems to be fine applied onto the current git master branch. Could probably be extracted just as with those changes that were in #5501.

@edolstra edolstra changed the title Optimize function/primop calls Merge environments of nested functions Nov 16, 2021
edolstra added a commit to edolstra/nix that referenced this pull request Nov 16, 2021
This is not really useful on its own, but it does recover the
'infinite recursion' error message for '{ __functor = x: x; } 1', and
is more efficient in conjunction with NixOS#3718.

Fixes NixOS#5515.
@edolstra
Copy link
Member Author

@andir Thanks, done in #5581.

dramforever pushed a commit to dramforever/nix that referenced this pull request Nov 27, 2021
This is not really useful on its own, but it does recover the
'infinite recursion' error message for '{ __functor = x: x; } 1', and
is more efficient in conjunction with NixOS#3718.

Fixes NixOS#5515.
@stale
Copy link

stale bot commented Jun 12, 2022

I marked this as stale due to inactivity. → More info

@stale stale bot added the stale label Jun 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants