shiv icon indicating copy to clipboard operation
shiv copied to clipboard

[Feature] Include `SHIV_COMPILE_WORKERS` in `environment.json`

Open sigmavirus24 opened this issue 9 months ago • 1 comments

Problem

tl;dr --compile-pyc is a potential footgun that is not well articulated in the documentation. It can provide legitimate benefits but to be entirely safe it needs the ability to put into place additional guardrails.

I've seen a number of teams at various organizations use shiv to make easy to distribute internal tooling. Once again, however, I've seen a team that distributed a tool and that tool was used on an environment that had 94 logical CPUs assigned to it. This caused shiv bootstrap (via compileall) to spin up 94 child processes for pyc compilation. This then caused the tool to hang without any visibility as it was part of a cron job because the preamble happens after extraction and compilation and this hang happens before then.

This is related to #54 and #200 but I'd like to propose a different solution since we've already turned on --no-compile-pyc to avoid this going forward.

Potential Solution

Allow the build stage to include a cap on the number of compilation workers. This is possible at execution time with SHIV_COMPILE_WORKERS but not every downstream consumer of a zipapp created by shiv will know:

  • That it needs to set this environment variable
  • That this zipapp was created with shiv instead of pex or another tool
  • That they're getting a Python zipapp executable

If this can be specified and stored in environment.json then this solves the problem of there being way more potential CPUs than there is work and a deadlock trying to compile the site-packages directory.

This also means that the person creating the zipapp can decide what might be a reasonable cap.

I would combine the storage of compile_workers in environment.json with a min(compile_workers, os.process_cpu_count()) so if the actual CPU count is lower than what was specified, we still use that instead of what was set.

Edit to add: I'm happy to send a PR if this seems reasonable.

sigmavirus24 avatar Jan 17 '25 16:01 sigmavirus24