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

No longer omit stdlib dependencies from dependency inference. #12818

Conversation

williamscs
Copy link
Contributor

Omitting these packages would break backport libraries (such as dataclasses). Performance impact is negligible since the dependency graph has already been gathered.

Benchmarks using
$ hyperfine --warmup=1 --runs=10 './pants --no-remote-cache-{read,write} --no-pantsd dependencies ::'

Before these changes (omitting stdlib dependencies):

Benchmark #1: ./pants --no-remote-cache-{read,write} --no-pantsd dependencies ::
  Time (mean ± σ):      8.852 s ±  0.105 s    [User: 8.612 s, System: 5.314 s]
  Range (min … max):    8.702 s …  9.107 s    10 runs

After changes (with stdlib dependency inference)

Benchmark #1: ./pants --no-remote-cache-{read,write} --no-pantsd dependencies ::
  Time (mean ± σ):      8.854 s ±  0.123 s    [User: 8.619 s, System: 5.333 s]
  Range (min … max):    8.682 s …  9.111 s    10 runs

[ci skip-rust]
[ci skip-build-wheels]

Omitting these packages would break backport libraries (such as dataclasses). Performance impact is negligible since the dependency graph has already been gathered.

Benchmarks using `$ hyperfine --warmup=1 --runs=10 './pants --no-remote-cache-{read,write} --no-pantsd dependencies ::'`
Before these changes (omitting stdlib dependencies):
```bash
Benchmark pantsbuild#1: ./pants --no-remote-cache-{read,write} --no-pantsd dependencies ::
  Time (mean ± σ):      8.852 s ±  0.105 s    [User: 8.612 s, System: 5.314 s]
  Range (min … max):    8.702 s …  9.107 s    10 runs
```

After changes (with stdlib dependency inference)
```bash
Benchmark pantsbuild#1: ./pants --no-remote-cache-{read,write} --no-pantsd dependencies ::
  Time (mean ± σ):      8.854 s ±  0.123 s    [User: 8.619 s, System: 5.333 s]
  Range (min … max):    8.682 s …  9.111 s    10 runs
```

[ci skip-rust]
[ci skip-build-wheels]
Copy link
Sponsor Member

@stuhood stuhood left a comment

Choose a reason for hiding this comment

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

Well, isn't that glorious?

@Eric-Arellano Eric-Arellano merged commit 4dfbe72 into pantsbuild:main Sep 9, 2021
@Eric-Arellano
Copy link
Contributor

Thanks so much @williamscs! FYI I merged with the title "Fix backports of std lib modules like dataclasses not working with dependency inference" - the PR title was already excellent, but changed so that we can categorize this as a "bug fix" in the changelog.

I'll cherry-pick to 2.7 now

Eric-Arellano pushed a commit to Eric-Arellano/pants that referenced this pull request Sep 9, 2021
…dependency inference (pantsbuild#12818)

Omitting these packages would break backport libraries (such as dataclasses). Performance impact is negligible since the dependency graph has already been gathered.

Benchmarks using 
`$ hyperfine --warmup=1 --runs=10 './pants --no-remote-cache-{read,write} --no-pantsd dependencies ::'`

Before these changes (omitting stdlib dependencies):
```bash
Benchmark #1: ./pants --no-remote-cache-{read,write} --no-pantsd dependencies ::
  Time (mean ± σ):      8.852 s ±  0.105 s    [User: 8.612 s, System: 5.314 s]
  Range (min … max):    8.702 s …  9.107 s    10 runs
```

After changes (with stdlib dependency inference)
```bash
Benchmark #1: ./pants --no-remote-cache-{read,write} --no-pantsd dependencies ::
  Time (mean ± σ):      8.854 s ±  0.123 s    [User: 8.619 s, System: 5.333 s]
  Range (min … max):    8.682 s …  9.111 s    10 runs
```

[ci skip-rust]
[ci skip-build-wheels]
Eric-Arellano added a commit that referenced this pull request Sep 9, 2021
…dependency inference (Cherry-pick of #12818) (#12819)

Omitting these packages would break backport libraries (such as dataclasses). Performance impact is negligible since the dependency graph has already been gathered.

Benchmarks using 
`$ hyperfine --warmup=1 --runs=10 './pants --no-remote-cache-{read,write} --no-pantsd dependencies ::'`

Before these changes (omitting stdlib dependencies):
```bash
Benchmark #1: ./pants --no-remote-cache-{read,write} --no-pantsd dependencies ::
  Time (mean ± σ):      8.852 s ±  0.105 s    [User: 8.612 s, System: 5.314 s]
  Range (min … max):    8.702 s …  9.107 s    10 runs
```

After changes (with stdlib dependency inference)
```bash
Benchmark #1: ./pants --no-remote-cache-{read,write} --no-pantsd dependencies ::
  Time (mean ± σ):      8.854 s ±  0.123 s    [User: 8.619 s, System: 5.333 s]
  Range (min … max):    8.682 s …  9.111 s    10 runs
```

[ci skip-rust]
[ci skip-build-wheels]
jsirois added a commit to jsirois/pants that referenced this pull request Sep 10, 2021
…uild#12778))

* Add python-docx to the module mapping dictionary ([pantsbuild#12775](pantsbuild#12775))

* Add python-pptx to the module mapping dictionary ([pantsbuild#12776](pantsbuild#12776))

* Add `opencv-python` to the default Python module mapping ([pantsbuild#12777](pantsbuild#12777))

* Add `PyMuPDF` to the default Python module mapping ([pantsbuild#12774](pantsbuild#12774))

* Deprecate `--list-provides` option. ([pantsbuild#12759](pantsbuild#12759))

* Upgrade default `isort` to latest `isort==5.9.3` ([pantsbuild#12756](pantsbuild#12756))

* `OutputPathField.value_or_default()` no longer has an `Address` argument ([pantsbuild#12837](pantsbuild#12837))

* Properly include file dependencies in docker build context ([pantsbuild#12758](pantsbuild#12758))

* DigestSubset should not short-circuit when there are ignores involved. ([pantsbuild#12648](pantsbuild#12648))

* Improve cache reuse for `./pants package` when using a constraints file or lockfile ([pantsbuild#12807](pantsbuild#12807))

* Upgrade to Pex 2.1.48 and leverage packed layout. ([pantsbuild#12808](pantsbuild#12808))

* Fix backports of std lib modules like `dataclasses` not working with dependency inference ([pantsbuild#12818](pantsbuild#12818))

* Warn if `[python-repos]` is set during lockfile generation ([pantsbuild#12800](pantsbuild#12800))

* Add `version` to lockfile metadata headers ([pantsbuild#12788](pantsbuild#12788))

* jvm: add missing space to avoid two words running together ([pantsbuild#12793](pantsbuild#12793))

* Fix a markdown issue in a help string. ([pantsbuild#12766](pantsbuild#12766))

[ci skip-rust]
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