Skip to content

Commit

Permalink
Add debug goals to python (#17057)
Browse files Browse the repository at this point in the history
See #17039.

Given a testbed of 
<details>
  <summary>input</summary>

```python
# Copyright 2022 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

import json  # unownable, root level
import os.path  # unownable, not root level

import watchdog  # dependency not included
import yaml  # dependency included
import yamlpath  # owned by other resolve

try:
    import weakimport  # weakimport missing
except ImportError:
    ...

open("src/python/configs/prod.json")  # asset
open("testprojects/pants-plugins/src/python/test_pants_plugin/__init__.py")
```
</details>

we get 

<details>
  <summary>output</summary>

```
{
  "src/python/pants/backend/python/dependency_inference/t.py": {
    "imports": [
      {
        "name": "weakimport",
        "reference": {
          "lineno": 12,
          "weak": true
        },
        "resolved": {
          "status": "ImportOwnerStatus.weak_ignore",
          "address": []
        },
        "possible_resolve": null
      },
      {
        "name": "json",
        "reference": {
          "lineno": 4,
          "weak": false
        },
        "resolved": {
          "status": "ImportOwnerStatus.unownable",
          "address": []
        },
        "possible_resolve": null
      },
      {
        "name": "os.path",
        "reference": {
          "lineno": 5,
          "weak": false
        },
        "resolved": {
          "status": "ImportOwnerStatus.unownable",
          "address": []
        },
        "possible_resolve": null
      },
      {
        "name": "watchdog",
        "reference": {
          "lineno": 7,
          "weak": false
        },
        "resolved": {
          "status": "ImportOwnerStatus.unowned",
          "address": []
        },
        "possible_resolve": null
      },
      {
        "name": "yaml",
        "reference": {
          "lineno": 8,
          "weak": false
        },
        "resolved": {
          "status": "ImportOwnerStatus.unambiguous",
          "address": [
            "3rdparty/python#PyYAML",
            "3rdparty/python#types-PyYAML"
          ]
        },
        "possible_resolve": null
      },
      {
        "name": "yamlpath",
        "reference": {
          "lineno": 9,
          "weak": false
        },
        "resolved": {
          "status": "ImportOwnerStatus.unowned",
          "address": []
        },
        "possible_resolve": [
          [
            "src/python/pants/backend/helm/subsystems:yamlpath",
            "helm-post-renderer"
          ]
        ]
      }
    ],
    "assets": [
      {
        "name": "src/python/configs/prod.json",
        "reference": "src/python/configs/prod.json",
        "resolved": {
          "status": "ImportOwnerStatus.unowned",
          "address": []
        },
        "possible_resolve": null
      },
      {
        "name": "testprojects/pants-plugins/src/python/test_pants_plugin/__init__.py",
        "reference": "testprojects/pants-plugins/src/python/test_pants_plugin/__init__.py",
        "resolved": {
          "status": "ImportOwnerStatus.unambiguous",
          "address": [
            "testprojects/pants-plugins/src/python/test_pants_plugin/__init__.py:../../../../pants_plugins_directory"
          ]
        },
        "possible_resolve": null
      }
    ]
  }
}
```
</details>

Telling you, for each file, for each import, what dependencies pants thought it could have, and what it decided to do with them.
This uses almost all the same code as the main dependency inference code, with the exception of the top-level orchestration of it. I think that's pretty close, there's about 100 lines of semi-duplicate code.

There's also a more advanced mode that dumps information about each stage of the process. I think this might be useful for people digging through the dependency inference process but not really for end-users. we get it for free, though.

Fixes #17039.

---

this is fairly critical for performance, so here are benchmarks (with comparison-of-means t-test)

|   | main | this | difference | P-score |
| --- | --- | --- | --- | --- |
| `hyperfine --runs=10 './pants --no-pantsd dependencies --transitive ::'` | 21.839 s ±  0.326 s | 22.142 s ±  0.283 s | 1.38% | 0.0395 |
| `hyperfine --warmup=1 --runs=10 './pants dependencies --transitive ::'` | 1.798 s ±  0.074 s | 1.811 s ±  0.076 s | 0.72% | 0.7029 |
| `hyperfine --runs=10 './pants --no-pantsd dependencies ::'` | 21.547 s ±  0.640 s  | 21.863 s ±  1.072 s | 1.47% | 0.4339 |
| `hyperfine --warmup=1 --runs=10 './pants dependencies ::'` | 1.828 s ±  0.091 s | 1.844 s ±  0.105 s | 0.88% | 0.7200 |

So it looks like this MR might impact performance, by about 1%, although those p-values are mighty unconvincing. LMK if we want to increase runs and get more statistics, I've run the stats a few times throughout and this looks about right, so I think we can proceed with the review under the assumption that there is currently a 1% performance overhead. I'm open to suggestions on improving performance.
  • Loading branch information
lilatomic authored Nov 17, 2022
1 parent 0420529 commit c446133
Show file tree
Hide file tree
Showing 8 changed files with 851 additions and 80 deletions.
3 changes: 2 additions & 1 deletion src/python/pants/backend/experimental/python/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from pants.backend.codegen import export_codegen_goal
from pants.backend.python.goals import publish
from pants.backend.python.goals import debug_goals, publish
from pants.backend.python.subsystems import setuptools_scm, twine
from pants.backend.python.target_types import VCSVersion
from pants.backend.python.util_rules import pex, vcs_versioning
Expand All @@ -16,6 +16,7 @@ def rules():
*setuptools_scm.rules(),
*export_codegen_goal.rules(),
*twine.rules(),
*debug_goals.rules(),
)


Expand Down
13 changes: 12 additions & 1 deletion src/python/pants/backend/project_info/peek.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from pants.engine.target import (
Dependencies,
DependenciesRequest,
Field,
HydratedSources,
HydrateSourcesRequest,
SourcesField,
Expand Down Expand Up @@ -89,12 +90,22 @@ class _PeekJsonEncoder(json.JSONEncoder):

def default(self, o):
"""Return a serializable object for o."""
if isinstance(o, str): # early exit prevents strings from being treated as sequences
return o
if o is None:
return o
if is_dataclass(o):
return asdict(o)
if isinstance(o, collections.abc.Mapping):
return dict(o)
if isinstance(o, collections.abc.Sequence):
if (
isinstance(o, collections.abc.Sequence)
or isinstance(o, set)
or isinstance(o, collections.abc.Set)
):
return list(o)
if isinstance(o, Field):
return self.default(o.value)
try:
return super().default(o)
except TypeError:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -384,8 +384,10 @@ def add_modules(modules: Iterable[str], *, type_stub: bool = False) -> None:
class PythonModuleOwners:
"""The target(s) that own a Python module.
If >1 targets own the same module, and they're implementations (vs .pyi type stubs), they will
be put into `ambiguous` instead of `unambiguous`. `unambiguous` should never be > 2.
Up to 2 targets can unambiguously own the same module, if one is an implementation and the other
is a .pyi type stub. It is ambiguous for >1 implementation target to own the same module, and
those targets will be put into `ambiguous` instead of `unambiguous`. Therefore, `unambiguous`
should never be >2; and only 1 of `unambiguous` and `ambiguous` should have targets.
"""

unambiguous: tuple[Address, ...]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
# Copyright 2020 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

import json
from dataclasses import dataclass

Expand Down
Loading

0 comments on commit c446133

Please sign in to comment.