From cfedd3b2123f36455e8523d03b6004601fccb932 Mon Sep 17 00:00:00 2001 From: gshuflin Date: Fri, 8 Jan 2021 16:19:50 -0800 Subject: [PATCH] Remove aggregated timings (#11431) This commit removes the last vestigial use of the `aggregated_timings` module from `RunTracker` and deletes the module. The `get_cumulative_timings` method has been modified to return the same JSON that would've been returned via the `aggregated_timings` mechanism. We will keep this around for backwards compatibility for the time being, but will likely implement a new API for getting timing data from `RunTracker` soon. This commit also adds a unit test for `RunTracker`. --- 3rdparty/python/constraints.txt | 6 ++- 3rdparty/python/requirements.txt | 1 + build-support/mypy/mypy.ini | 3 ++ src/python/pants/goal/BUILD | 3 ++ src/python/pants/goal/aggregated_timings.py | 47 --------------------- src/python/pants/goal/run_tracker.py | 16 +++---- src/python/pants/goal/run_tracker_test.py | 27 ++++++++++++ 7 files changed, 43 insertions(+), 60 deletions(-) delete mode 100644 src/python/pants/goal/aggregated_timings.py create mode 100644 src/python/pants/goal/run_tracker_test.py diff --git a/3rdparty/python/constraints.txt b/3rdparty/python/constraints.txt index 84208a87155..a7b9da2065c 100644 --- a/3rdparty/python/constraints.txt +++ b/3rdparty/python/constraints.txt @@ -1,4 +1,4 @@ -# Generated by build-support/bin/generate_lockfile.sh on Wed Dec 23 10:11:46 PST 2020 +# Generated by build-support/bin/generate_lockfile.sh on Thu Jan 7 05:28:27 PM PST 2021 ansicolors==1.1.8 attrs==20.3.0 beautifulsoup4==4.6.3 @@ -7,6 +7,7 @@ cffi==1.14.4 chardet==4.0.0 cryptography==3.3.1 fasteners==0.15 +freezegun==1.0.0 idna==2.10 iniconfig==1.1.1 monotonic==1.5 @@ -24,12 +25,13 @@ pyOpenSSL==20.0.1 pyparsing==2.4.7 pystache==0.5.4 pytest==6.2.1 +python-dateutil==2.8.1 PyYAML==5.3.1 requests==2.25.1 setproctitle==1.2 setuptools==50.3.2 six==1.15.0 toml==0.10.2 -typed-ast==1.4.1 +typed-ast==1.4.2 typing-extensions==3.7.4.2 urllib3==1.26.2 diff --git a/3rdparty/python/requirements.txt b/3rdparty/python/requirements.txt index 88f5c9b2b4a..8780f16e3d3 100644 --- a/3rdparty/python/requirements.txt +++ b/3rdparty/python/requirements.txt @@ -1,6 +1,7 @@ ansicolors==1.1.8 beautifulsoup4>=4.6.0,<4.7 fasteners==0.15.0 +freezegun==1.0.0 # The MyPy requirement should be maintained in lockstep with the requirement the Pants repo uses # for the mypy task since it configures custom MyPy plugins. That requirement can be found via: diff --git a/build-support/mypy/mypy.ini b/build-support/mypy/mypy.ini index 8f7f850b224..a96145a2024 100644 --- a/build-support/mypy/mypy.ini +++ b/build-support/mypy/mypy.ini @@ -81,3 +81,6 @@ ignore_missing_imports = True [mypy-www_authenticate] ignore_missing_imports = True + +[mypy-freezegun] +ignore_missing_imports = True diff --git a/src/python/pants/goal/BUILD b/src/python/pants/goal/BUILD index 875493cb7c6..233cadab251 100644 --- a/src/python/pants/goal/BUILD +++ b/src/python/pants/goal/BUILD @@ -2,3 +2,6 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). python_library() +python_tests( + name='tests' +) diff --git a/src/python/pants/goal/aggregated_timings.py b/src/python/pants/goal/aggregated_timings.py deleted file mode 100644 index bfd5d84e99f..00000000000 --- a/src/python/pants/goal/aggregated_timings.py +++ /dev/null @@ -1,47 +0,0 @@ -# Copyright 2014 Pants project contributors (see CONTRIBUTORS.md). -# Licensed under the Apache License, Version 2.0 (see LICENSE). - -import os -from collections import defaultdict -from typing import Dict, List, Union - -from pants.util.dirutil import safe_mkdir_for - -TimingData = List[Dict[str, Union[str, float, bool]]] - - -class AggregatedTimings: - """Aggregates timings over multiple invocations of 'similar' work. - - If filepath is not none, stores the timings in that file. Useful for finding bottlenecks. - """ - - def __init__(self, path: str) -> None: - # Map path -> timing in seconds (a float) - self._timings_by_path: Dict[str, float] = defaultdict(float) - self._path = path - safe_mkdir_for(path) - - def add_timing(self, label: str, secs: float) -> None: - """Aggregate timings by label. - - secs - a double, so fractional seconds are allowed. - """ - self._timings_by_path[label] += secs - if not os.path.exists(os.path.dirname(self._path)): - # Check existence in case we're a clean-all. We don't want to write anything in that case. - return - with open(self._path, "w") as fl: - for timing_row in self.get_all(): - fl.write("{label}: {timing}\n".format(**timing_row)) - - def get_all(self) -> TimingData: - """Returns all the timings, sorted in decreasing order. - - Each value is a dict: { path: , timing: } - """ - # The "is_tool" key is legacy, and is only kept around for the moment to support tools that might expect it. - return [ - {"label": row[0], "timing": row[1], "is_tool": False} - for row in sorted(self._timings_by_path.items(), key=lambda x: x[1], reverse=True) - ] diff --git a/src/python/pants/goal/run_tracker.py b/src/python/pants/goal/run_tracker.py index b6a908dd51c..0c4b76e0ae3 100644 --- a/src/python/pants/goal/run_tracker.py +++ b/src/python/pants/goal/run_tracker.py @@ -9,12 +9,11 @@ import uuid from collections import OrderedDict from pathlib import Path -from typing import Dict, List, Optional +from typing import Any, Dict, List, Optional from pants.base.exiter import PANTS_SUCCEEDED_EXIT_CODE, ExitCode from pants.base.run_info import RunInfo from pants.engine.internals.native import Native -from pants.goal.aggregated_timings import AggregatedTimings, TimingData from pants.option.config import Config from pants.option.options import Options from pants.option.options_fingerprinter import CoercingOptionEncoder @@ -95,11 +94,6 @@ def __init__(self, options: Options): self.run_info_dir = os.path.join(info_dir, self.run_id) self.run_info = RunInfo(os.path.join(self.run_info_dir, "info")) - # Time spent in a workunit, including its children. - self.cumulative_timings = AggregatedTimings( - os.path.join(self.run_info_dir, "cumulative_timings") - ) - # pantsd stats. self._pantsd_metrics: Dict[str, int] = dict() @@ -108,6 +102,7 @@ def __init__(self, options: Options): # Initialized in `start()`. self._run_start_time: Optional[float] = None + self._run_total_duration: Optional[float] = None @property def goals(self) -> List[str]: @@ -185,8 +180,7 @@ def end_run(self, exit_code: ExitCode) -> None: raise Exception("RunTracker.end_run() called without calling .start()") duration = time.time() - self._run_start_time - - self.cumulative_timings.add_timing(label="main", secs=duration) + self._total_run_time = duration outcome_str = "SUCCESS" if exit_code == PANTS_SUCCEEDED_EXIT_CODE else "FAILURE" @@ -200,8 +194,8 @@ def end_run(self, exit_code: ExitCode) -> None: return - def get_cumulative_timings(self) -> TimingData: - return self.cumulative_timings.get_all() + def get_cumulative_timings(self) -> List[Dict[str, Any]]: + return [{"label": "main", "timing": self._total_run_time}] def get_options_to_record(self) -> dict: recorded_options = {} diff --git a/src/python/pants/goal/run_tracker_test.py b/src/python/pants/goal/run_tracker_test.py new file mode 100644 index 00000000000..dd80fc4af8c --- /dev/null +++ b/src/python/pants/goal/run_tracker_test.py @@ -0,0 +1,27 @@ +# Copyright 2021 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). + +import datetime +import time + +from freezegun import freeze_time + +from pants.base.exiter import PANTS_SUCCEEDED_EXIT_CODE +from pants.goal.run_tracker import RunTracker +from pants.testutil.option_util import create_options_bootstrapper +from pants.util.contextutil import environment_as, temporary_dir + + +@freeze_time(datetime.datetime(2020, 1, 1, 12, 0, 0), as_kwarg="frozen_time") +def test_run_tracker_timing_output(**kwargs) -> None: + with temporary_dir() as buildroot: + with environment_as(PANTS_BUILDROOT_OVERRIDE=buildroot): + run_tracker = RunTracker(create_options_bootstrapper([]).bootstrap_options) + run_tracker.start(run_start_time=time.time()) + frozen_time = kwargs["frozen_time"] + frozen_time.tick(delta=datetime.timedelta(seconds=1)) + run_tracker.end_run(PANTS_SUCCEEDED_EXIT_CODE) + + timings = run_tracker.get_cumulative_timings() + assert timings[0]["label"] == "main" + assert timings[0]["timing"] == 1.0