addToStore(): Evaluate on the main stack

[edolstra]

Motivation

Switching from sinkToSource() to sourceToSink() causes the path filter evaluation to happen on the thread's main stack rather than on the coroutine stack. This should avoid the need for our Boehm GC coroutine workarounds, since the GC roots will be on the main stack of the thread.

Fixes #11141.

Context

Priorities and Process

Add 👍 to pull requests you find important.

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

[nixos-discourse]

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

https://discourse.nixos.org/t/2024-07-22-nix-team-meeting-minutes-163/49544/1

[edolstra]

Team discussion:

• Need to benchmark that this doesn't introduce a performance regression.
• Otherwise looks good.

[tomberek]

# started on Sat Aug 17 00:36:50 2024


 Performance counter stats for './run-result.sh' (4 runs):

          4,747.09 msec task-clock:u                     #    0.998 CPUs utilized               ( +-  0.11% )
                 0      context-switches:u               #    0.000 /sec                      
                 0      cpu-migrations:u                 #    0.000 /sec                      
           190,199      page-faults:u                    #   40.066 K/sec                       ( +-  0.00% )
    12,047,705,665      cycles:u                         #    2.538 GHz                         ( +-  0.16% )
    22,541,252,252      instructions:u                   #    1.87  insn per cycle              ( +-  0.02% )
     4,413,428,462      branches:u                       #  929.712 M/sec                       ( +-  0.02% )
        74,194,689      branch-misses:u                  #    1.68% of all branches             ( +-  0.12% )
                        TopdownL1                 #     16.2 %  tma_backend_bound      
                                                  #     41.4 %  tma_bad_speculation    
                                                  #     15.8 %  tma_frontend_bound     
                                                  #     26.6 %  tma_retiring             ( +-  0.20% )

           # Table of individual measurements:
           4.75286 (-0.00515) #
           4.74741 (-0.01060) #
           4.76610 (+0.00809) #
           4.76568 (+0.00767) #

           # Final result:
           4.75801 +- 0.00468 seconds time elapsed  ( +-  0.10% )

# started on Sat Aug 17 00:36:05 2024


 Performance counter stats for './run-stable.sh' (4 runs):

          5,041.44 msec task-clock:u                     #    0.998 CPUs utilized               ( +-  0.44% )
                 0      context-switches:u               #    0.000 /sec                      
                 0      cpu-migrations:u                 #    0.000 /sec                      
           189,840      page-faults:u                    #   37.656 K/sec                       ( +-  0.01% )
    12,857,410,437      cycles:u                         #    2.550 GHz                         ( +-  0.38% )
    25,654,027,530      instructions:u                   #    2.00  insn per cycle              ( +-  0.04% )
     4,689,079,121      branches:u                       #  930.106 M/sec                       ( +-  0.03% )
        79,691,381      branch-misses:u                  #    1.70% of all branches             ( +-  0.23% )
                        TopdownL1                 #     17.2 %  tma_backend_bound      
                                                  #     37.0 %  tma_bad_speculation    
                                                  #     14.9 %  tma_frontend_bound     
                                                  #     30.9 %  tma_retiring             ( +-  0.14% )

            # Table of individual measurements:
            5.1002 (+0.0494) #
            5.0718 (+0.0210) #
            5.0440 (-0.0069) #
            4.9874 (-0.0635) #

            # Final result:
            5.0508 +- 0.0241 seconds time elapsed  ( +-  0.48% )

[tomberek]

This does make evaluation with GC slightly worse, but greatly simplifies implementation. Performance cost seems low.

I could not update this branch, but have a rebase at: https://github.com/tomberek/nix/tree/flip-coroutines

[edolstra]

I also measured and got a slight (not statistically significant) speedup:

elapsed time:       median =     18.8708  mean =     18.8354  stddev =      0.0964  min =     18.6845  max =     18.9424  [not rejected, p=0.10362, Δ=-0.10630±0.20030]

[github-actions]

Backport failed because this pull request contains merge commits. You can either backport this pull request manually, or configure the action to skip merge commits.

[Mic92]

Does this mean we can get rid of 5740924 enableLargeConfig = true?