-
-
Notifications
You must be signed in to change notification settings - Fork 690
Description
In these examples /home/ecsb/src/o/alt-pants/pants is checked out to the 2.31.x branch @ 29576ba and I've set PY= to use the same Python Build Standalone as scie-pants to try to minimize differences. I'm running these commands in a repo with a few thousand python files (https://github.com/cburroughs/clam-diggers/)
How long does our release build take?
$ time PANTS_DEBUG= _PANTS_VERSION_OVERRIDE=2.31.0 /home/ecsb/src/o/alt-pants/pants --no-pantsd dependencies :: > /dev/null
Pantsd has been turned off via Flag.
real 0m17.124s
user 0m24.807s
sys 0m6.637s
Great, got a good baseline. So when I run this from source I should get the same thing? I don't. Much flailing about with Python optimization flags, cargo, and what PBS does ensures, for the purposes of this ticket we are going to skip to where I am using PBS with the pants in-repo script by way of PY=. I'm literally using the PBS from ~/.cache/nce so nothing can be different, right?
$ time _PANTS_VERSION_OVERRIDE=2.31.0 /home/ecsb/src/o/alt-pants/pants --no-pantsd dependencies :: > /dev/null
Pantsd has been turned off via Flag.
real 0m28.273s
user 0m31.862s
sys 0m13.343s
But, what if we enable debugging but remove the debugger?
~/.cache/pants/pants_dev_deps/6343a8ac7914ba89d9425c32e39273681a521e58.venv/bin/pip uninstall debugpy
Found existing installation: debugpy 1.8.16
Uninstalling debugpy-1.8.16:
Would remove:
/home/ecsb/.cache/pants/pants_dev_deps/6343a8ac7914ba89d9425c32e39273681a521e58.venv/bin/debugpy
/home/ecsb/.cache/pants/pants_dev_deps/6343a8ac7914ba89d9425c32e39273681a521e58.venv/bin/debugpy-adapter
/home/ecsb/.cache/pants/pants_dev_deps/6343a8ac7914ba89d9425c32e39273681a521e58.venv/lib/python3.11/site-packages/debugpy-1.8.16.dist-info/*
/home/ecsb/.cache/pants/pants_dev_deps/6343a8ac7914ba89d9425c32e39273681a521e58.venv/lib/python3.11/site-packages/debugpy/*
Proceed (Y/n)? y
Successfully uninstalled debugpy-1.8.16
$ time PANTS_DEBUG= _PANTS_VERSION_OVERRIDE=2.31.0 /home/ecsb/src/o/alt-pants/pants --no-pantsd dependencies :: > /dev/null
Pantsd has been turned off via Flag.
real 0m17.124s
user 0m24.807s
sys 0m6.637s
That's fast! Cool, but it PANTS_DEBUG= load bearing? We did just remove the debugger right?
$ time _PANTS_VERSION_OVERRIDE=2.31.0 /home/ecsb/src/o/alt-pants/pants --no-pantsd dependencies :: > /dev/null
Pantsd has been turned off via Flag.
real 0m29.633s
user 0m32.261s
sys 0m19.453s
😕
What does PANTS_DEBUG= even do?
pants/src/rust/task_executor/src/lib.rs
Lines 93 to 95 in f07276d
| if env::var("PANTS_DEBUG").is_ok() { | |
| runtime_builder.on_thread_start(on_thread_start); | |
| }; |
pants/src/rust/engine/src/externs/scheduler.rs
Lines 30 to 43 in f07276d
| task_executor::Executor::new_owned(core_threads, max_threads, || { | |
| // NB: We need a PyThreadState object which lives throughout the lifetime of this thread | |
| // as the debug trace object is attached to it. Otherwise the PyThreadState is | |
| // constructed/destroyed with each `with_gil` call (inside PyGILState_Ensure/PyGILState_Release). | |
| // | |
| // Constructing (and leaking) a ThreadState object allocates and associates it with the current | |
| // thread, and the Python runtime won't wipe the trace function between calls. | |
| // See https://github.com/PyO3/pyo3/issues/2495 | |
| let _ = | |
| unsafe { ffi::PyThreadState_New(Python::attach(|_| PyInterpreterState_Main())) }; | |
| Python::attach(|py| { | |
| let _ = py.eval(c"__import__('debugpy').debug_this_thread()", None, None); | |
| }); | |
| }) |
And to circle back to the top, what is scie-pants doing:
PANTS_VERSION=2.31.0 pants --no-pantsd help 2>/dev/null & sleep 1; grep -z PANTS_DEBUG /proc/$!/environ | tr '\0' '\n'; kill $!
[1] 23985
PANTS_DEBUG=
Recommendations:
- I don't fully grok all the
PyThreadState_Newconsequences. But if that is faster, how we have been running since scie-pants, and needed for debugging maybe we should always do that. PANTS_DEBUG=meaning "true" is not what I at least would expect. But that might be hard to unwind given it's historically load bearing nature.