fs.Snapshot is declared in Rust #11328
Conversation
# Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
…antsbuild#10905. # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
| @@ -13,6 +13,15 @@ class PyDigest: | |||
| @property | |||
| def serialized_bytes_length(self) -> int: ... | |||
|
|
|||
| class PySnapshot: | |||
| def __init__(self) -> None: ... | |||
There was a problem hiding this comment.
This is surprising to me - is it no longer possible to use the constructor with 3 args?
There was a problem hiding this comment.
It is ~impossible to create a correct Snapshot other than "the empty snapshot" by hand, because you need assistance computing the correct Digest for the files. Consequently, that was the only call to the Snapshot constructor on the Python side.
In every profile that I look at, lock contention (on the GIL and elsewhere) shows up as our largest bottleneck. While there is more to do around optimizing our locking, the only way to reduce the GIL time is to interact with it less (which in some cases might mean holding it for a longer period rather than acquiring it N times). |
While we're unlikely to do it, I am still curious how much faster Pants would be if it were purely implemented in Rust. I wonder if it would be feasible to someday expose the plugin API via Rust. (Ack that it would be substantially less accessible, and likely not worth it.) |
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.Digestto Rust in #10905 missed the optimization opportunity of directly extracting the inner RustDigestinstance.Solution
In two commits: port
fs.Snapshotto Rust, and optimize the conversion of aPyDigestto aDigest.Result
6% faster runs of
./pants dependencies --transitive ::.[ci skip-build-wheels]