Skip to content

Perf: SourceField, PathGlobs and Filespec in rust#23244

Merged
tobni merged 9 commits into
pantsbuild:mainfrom
tobni:add/source-field-in-rust
Jun 4, 2026
Merged

Perf: SourceField, PathGlobs and Filespec in rust#23244
tobni merged 9 commits into
pantsbuild:mainfrom
tobni:add/source-field-in-rust

Conversation

@tobni
Copy link
Copy Markdown
Contributor

@tobni tobni commented Apr 12, 2026

hyperfine --warmup 1 -n 'main' 'PYENV_VERSION=pants@3.14.3 PANTS_SOURCE=/tmp/pants-main pants --no-pantsd dependencies ::' -n 'add/source-field-in-rust' 'PYENV_VERSION=pants@3.14.3 PANTS_SOURCE=<redacted>/pants pants --no-pantsd dependencies ::'
Benchmark 1: main
  Time (mean ± σ):     32.467 s ±  0.416 s    [User: 37.098 s, System: 11.984 s]
  Range (min … max):   31.933 s … 33.181 s    10 runs
 
Benchmark 2: add/source-field-in-rust
  Time (mean ± σ):     30.605 s ±  0.456 s    [User: 35.140 s, System: 11.875 s]
  Range (min … max):   30.097 s … 31.519 s    10 runs
 
Summary
  add/source-field-in-rust ran
    1.06 ± 0.02 times faster than main

@tobni tobni added category:performance release-notes:not-required [CI] PR doesn't require mention in release notes labels Apr 12, 2026
@tobni tobni force-pushed the add/source-field-in-rust branch 2 times, most recently from d380619 to 6334dad Compare April 12, 2026 13:02
getattr(cls, "PartitionRequest"): cls._requires_snapshot for cls in _all_lint_requests()
}[type(request)]
if operates_on_paths:
return Partitions.single_partition(fs.sources.globs for fs in request.field_sets)
Copy link
Copy Markdown
Contributor Author

@tobni tobni Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passed tuples to something expecting strings. Pyo3 validation of types catches it.

#[derive(Debug)]
pub struct Globs {
strings: Box<[String]>,
py_tuple: OnceLock<Py<PyTuple>>,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

memoizing these may be design overkill, but userland could suffer if their plugin reads globs in python and we copy to python heap every access.

Comment thread src/rust/engine/src/externs/fs.rs
@tobni tobni force-pushed the add/source-field-in-rust branch 2 times, most recently from 8dbcaec to f61bb10 Compare April 12, 2026 14:17
Copy link
Copy Markdown
Contributor

@benjyw benjyw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me overall! Could use more testing, esp of the functions claiming to emulate Python functionality. It would be good to have tests that run that function via the ffi and run the Python equivalent and compare outputs, across a large number of inputs.

Comment thread src/rust/engine/src/externs/target/util.rs Outdated
Comment thread src/rust/engine/src/externs/target/util.rs
Comment thread src/rust/engine/src/externs/target/util.rs Outdated
Comment thread src/rust/engine/src/externs/fs.rs
@tobni tobni force-pushed the add/source-field-in-rust branch from f61bb10 to 3ada403 Compare May 30, 2026 16:06
@tobni
Copy link
Copy Markdown
Contributor Author

tobni commented May 30, 2026

@benjyw I rebased ontop of main since I had conflicts.
Only change is in 4bd44cd which I think addresses your comments

@tobni tobni force-pushed the add/source-field-in-rust branch from 3ada403 to 4bd44cd Compare May 30, 2026 16:17
@tobni tobni merged commit 95da181 into pantsbuild:main Jun 4, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:performance release-notes:not-required [CI] PR doesn't require mention in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants