Make EvalState::callDepth thread-local

[edolstra]

Motivation

This is needed to make it thread-safe.

Context

---

Add 👍 to pull requests you find important.

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

[Ericson2314]

Why is it a static? That seems bad if there are multiple interpreters

[xokdvium]

That seems bad if there are multiple interpreters

Yeah, this is a non-starter fundamentally. This would really conflict with the multi-evaluator goals (cc @roberth). We should make code more reusable and without globals.

I think the idea here is to make the call depth local to an evaluation worker (by abusing thread_local mechanisms). Wouldn't it be better to instead have that concept expressed as an evaluation context of sorts?

[edolstra]

Why is it a static? That seems bad if there are multiple interpreters

Why? Presumably any thread will have only one interpreter active at any given time. Or, in the unlikely scenario that one interpreter calls another, they should share the same callDepth variable since recursion depth is the sum of all active calls.

This would really conflict with the multi-evaluator goals

What's the use case for that?

[roberth]

Multithreading creates a situation where callDepth is even less deterministic than it used to be. Imagine thunk A is strict in B, which is strict in C. If thread 1 wants A and thread 2 wants B, and thread A is near its limit, the execution ordering between 1 and 2 will decide whether C is evaluated in 1, where it fails, or 2, where it succeeds.

In other words callDepth will fail nondeterministically, which defeats its purpose.

I'm not a big fan of callDepth anyway. IMO it should behave no differently than any other memory consuming behavior. Nothing in the language semantics dictates this, and it puts unnecessary restrictions on the kinds of code that can be written.
It can be removed in two ways I'm aware of

• turn thunk evaluation from an if is thunk to while is thunk, so that evaluation work can be resumed on an intermediate thunk after returning, much to the effect of a tail call
• jump to a new coroutine stack once we're near the end of the current stack space

The prior is more memory efficient but also produces less detailed error traces, as the return terminates the trys. The latter is less invasive in terms of code changes.

That all is in addition to worries that I share about the reduced flexibility. This could affect the unit tests, as well as C API users, and it may make experimentation with the evaluator more difficult, e.g. new caching approaches or agent-style isolated heaps per flake.