Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the pants native-client if it is present in the distribution. #172

Merged
merged 6 commits into from
May 15, 2023

Conversation

stuhood
Copy link
Sponsor Member

@stuhood stuhood commented May 8, 2023

This change uses the native-client binary included in pantsbuild/pants wheels by pantsbuild/pants#18957 (and further adjusted in pantsbuild/pants#19010) to launch pants. See that change for the performance impact.

@stuhood stuhood marked this pull request as ready for review May 9, 2023 20:38
@stuhood stuhood requested review from kaos and benjyw May 9, 2023 20:39
stuhood added a commit to pantsbuild/pants that referenced this pull request May 10, 2023
The Pants native client which was introduced in #11922 has so far only
been usable in the Pants repo when the `USE_NATIVE_PANTS` environment
variable was set.

To make the native client available to end users, this change begins
distributing the native-client binary in Pants wheels. A corresponding
change in the `scie-pants` repo
(pantsbuild/scie-pants#172) uses the native
client to run `pants`.

To reduce the surface area of `scie-pants` (rather than having it be
responsible for handling the special `75` exit code similar to the
`pants` script integration), this PR also moves to having the
native-client execute its own fallback (via `execv`) to the Pants
entrypoint. In future, the `pantsbuild/pants` `pants` script could also
use that facility (e.g. by specifying a separate `pants_server` bash
script as the entrypoint to use as the `_PANTS_SERVER_EXE`).

----

As originally demonstrated on #11831, the native client is still much
faster than the legacy client. Using
pantsbuild/scie-pants#172, the timings look
like:
```
Benchmark #1: PANTS_NO_NATIVE_CLIENT=true PANTS_SHA=836cceb74e6af042e7c82677f3ceb4927efce20e scie-pants-macos-x86_64 help
  Time (mean ± σ):      1.161 s ±  0.067 s    [User: 830.6 ms, System: 79.2 ms]
  Range (min … max):    1.054 s …  1.309 s    10 runs

Benchmark #2: PANTS_SHA=836cceb74e6af042e7c82677f3ceb4927efce20e scie-pants-macos-x86_64 help
  Time (mean ± σ):     271.0 ms ±  30.6 ms    [User: 8.9 ms, System: 6.9 ms]
  Range (min … max):   241.5 ms … 360.6 ms    12 runs

Summary
  'PANTS_SHA=836cceb74e6af042e7c82677f3ceb4927efce20e scie-pants-macos-x86_64 help' ran
    4.29 ± 0.54 times faster than 'PANTS_NO_NATIVE_CLIENT=true PANTS_SHA=836cceb74e6af042e7c82677f3ceb4927efce20e scie-pants-macos-x86_64 help'
```

Fixes #11831.
tools/src/scie_pants/install_pants.py Outdated Show resolved Hide resolved
package/scie-pants.lift.json Outdated Show resolved Hide resolved
package/scie-pants.lift.json Outdated Show resolved Hide resolved
@stuhood
Copy link
Sponsor Member Author

stuhood commented May 15, 2023

Adjusted for pantsbuild/pants#19010

@jsirois
Copy link
Contributor

jsirois commented May 15, 2023

If you want to roll in the version bump & CHANGES entry, this could go straight to release.

package/scie-pants.lift.json Show resolved Hide resolved
stuhood added a commit to pantsbuild/pants that referenced this pull request May 15, 2023
On pantsbuild/scie-pants#172, @jsirois pointed
out that allowing the native-client to handle disabling itself (if
requested) would be much cleaner from a `scie-pants` perspective, and
keeping `scie-pants` as small and simple as possible is an important
goal.

This change moves `PANTS_NO_NATIVE_CLIENT` handling into the native
client.
@stuhood stuhood merged commit 750ccd7 into pantsbuild:main May 15, 2023
@stuhood stuhood deleted the stuhood/native-client branch May 15, 2023 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants