fs.Digest is declared in Rust #10905
Conversation
[ci skip-build-wheels]
|
LGTM.
I don't see how that's significant. The universe of intrinsic types will be small by definition. This is a disadvantage we should be willing to put up with since it really only affects a few core developers unlike... The only real disadvantage I can see would be losing easy to find / idiomatic API information for rule authors. That's already non-ideal since there is no easy way to know what intrinsic input -> output mappings there are without looking at high level docs. Here we lose a natural place to hang field and method docs for data types (unless ecosystem tooling knows to pick docs off .pyi stubs when otherwise missing?). These are clearly solvable problems though. |
There was a problem hiding this comment.
Excellent! I agree with John. My main concern was that users can no longer look at engine/fs.py to see a simple dataclass to see how the type is implemented. The type stubs mitigate my concerns.
I'm not too concerned about the custom eq and hash. Sure, it's some boilerplate, but we had lots of boilerplate to begin with.
| sources=['native.py'], | ||
| sources=[ | ||
| 'native.py', | ||
| 'native_engine.pyi', |
There was a problem hiding this comment.
Woot! This is exciting to see used.
| product=get.output_type, | ||
| declared_subject=get.input_type, | ||
| subject=get.input, |
There was a problem hiding this comment.
Hm, fine to not fix, but I thought I had fixed most of the Rust code to call this input and output.
# Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
|
Reviewable. |
Internal only changes left off from the changelog: * Use cpython types in Rust functions that manipulate python objects (#10942) `PR #10942 <https://github.com/pantsbuild/pants/pull/10942>`_ * update libz-sys version to fix macOS compile error (#10941) `PR #10941 <https://github.com/pantsbuild/pants/pull/10941>`_ * Upgrade to Rust stable 1.47.0. (#10933) `PR #10933 <https://github.com/pantsbuild/pants/pull/10933>`_ * Finish CreateDigest Directory cleanup. (#10935) `PR #10935 <https://github.com/pantsbuild/pants/pull/10935>`_ * Hotfix broken import from merge conflict (#10934) `PR #10934 <https://github.com/pantsbuild/pants/pull/10934>`_ * Revert "Port nailgun client to rust (#10865)" (#10929) `PR #10929 <https://github.com/pantsbuild/pants/pull/10929>`_ * An ExternalTool for downloading the grpc_python_plugin. (#10927) `PR #10927 <https://github.com/pantsbuild/pants/pull/10927>`_ * Port nailgun client to rust (#10865) `PR #10865 <https://github.com/pantsbuild/pants/pull/10865>`_ * print stacktraces during import errors (#10906) `PR #10906 <https://github.com/pantsbuild/pants/pull/10906>`_ * fs.Digest is declared in Rust (#10905) `PR #10905 <https://github.com/pantsbuild/pants/pull/10905>`_ * add requests_ca_bundle to settable_env_vars (#10909) `PR #10909 <https://github.com/pantsbuild/pants/pull/10909>`_ [ci skip-rust]
…antsbuild#10905. # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
### Problem Our fundamental filesystem types are declared in Python, but declaring them in Rust would cut down on FFI costs, and allow for better error messages by removing some usage of the `externs::getattr*` functions. Additionally, the previous change to move `fs.Digest` to Rust in #10905 missed the optimization opportunity of directly extracting the inner Rust `Digest` instance. ### Solution In two commits: port `fs.Snapshot` to Rust, and optimize the conversion of a `PyDigest` to a `Digest`. ### Result 6% faster runs of `./pants dependencies --transitive ::`. [ci skip-build-wheels]
|
|
||
| fingerprint: str | ||
| serialized_bytes_length: int | ||
| You can use `await Get(Snapshot, Digest)` to set the file names referred to, or use `await |
Problem
Our fundamental filesystem types are declared in Python, but declaring them in Rust would cut down on FFI costs, and allow for better error messages by removing some usage of the
externs::project_*functions.Solution
Port
fs.Digestto Rust. An advantage in the case ofDigestis that unless the Python side callsDigest.fingerprint, we'll never bother to encode the fingerprint as hex (it stays binary in the privatehashing::Digestinstance. And likewise, if the engine is given back aDigestobject that it had previously given out, it doesn't have to decode hex at the boundary. While it would have been possible to do this with Python code (by having a privatebytesfield for the fingerprint rather than a string), it would have involved redundant logic for validation.Additionally adds initial Python typestubs for the
native_enginemodule, which will likely be the eventual home for documentation on native types.Result
For 100 runs: