From 29d56d3d654862dfaa4c551544b2271a6145ea06 Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Sun, 14 Apr 2024 22:45:31 -0700 Subject: [PATCH 1/5] misc,tests: Add Pyunit tests to CI GitHub Action Workflow Due to an oversight, the PyUnit tests were not being run as part of the gem5 CI tests. This was because they are located in "tests/pyunit" instead of "tests/gem5", where the CI GitHub Action workflow searched for tests to run and where all other tests reside. This adds the Pyunit tests as a seperate job in the CI GitHub Action's workflow. Change-Id: I63d93571fde11c19bf3d281c034eddf4b455ae4e --- .github/workflows/ci-tests.yaml | 39 ++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci-tests.yaml b/.github/workflows/ci-tests.yaml index 55b10dbb6b..a1aebe82ac 100644 --- a/.github/workflows/ci-tests.yaml +++ b/.github/workflows/ci-tests.yaml @@ -184,12 +184,49 @@ jobs: path: tests/testing-results retention-days: 30 + pyunit: + runs-on: [self-hosted, linux, x64] + if: github.event.pull_request.draft == false + container: ghcr.io/gem5/ubuntu-24.04_all-dependencies:latest + needs: [pre-commit, check-for-change-id, testlib-quick-gem5-builds] + timeout-minutes: 30 + steps: + + # Checkout the repository then download the builds. + - uses: actions/checkout@v4 + - uses: actions/download-artifact@v4 + with: + name: ci-tests-${{ github.run_number }}-testlib-quick-all-gem5-builds + + # Check that the gem5 binaries exist and are executable. + - name: Chmod gem5.{opt,debug,fast} to be executable + run: | + find . -name "gem5.opt" -exec chmod u+x {} \; + find . -name "gem5.debug" -exec chmod u+x {} \; + find . -name "gem5.fast" -exec chmod u+x {} \; + + # Run the pyunit tests. + # Note: these are all quick tests. + - name: Run The pyunit tests + id: run-tests + working-directory: ${{ github.workspace }}/tests + run: ./main.py run --skip-build -vv -j$(nproc) pyunit + + # Upload the tests/testing-results directory as an artifact. + - name: Upload pyunit test results + if: success() || failure() + uses: actions/upload-artifact@v4 + with: + name: ci-tests-run-${{ github.run_number }}-attempt-${{ github.run_attempt }}-pyunit-status-${{ steps.run-tests.outcome }}-output + path: tests/testing-results + retention-days: 30 + testlib-quick: # It is 'testlib-quick' which needs to pass for the pull request to be # merged. The 'testlib-quick-execution' is a matrix job which runs all the # the testlib quick tests. This job is therefore a stub which will pass if # all the testlib-quick-execution jobs pass. runs-on: ubuntu-latest - needs: testlib-quick-execution + needs: [testlib-quick-execution, pyunit] steps: - run: echo "This job's status is ${{ job.status }}." From e4ff5df35a74dd7f4cb726373f03e6b6aea28278 Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Thu, 18 Apr 2024 12:20:48 -0700 Subject: [PATCH 2/5] tests,stdlib: Fix pyunit tests - Workload -> ShadowResource Change-Id: I307439334c93851ebe3a78d3a80d048374a0900a --- tests/pyunit/stdlib/resources/pyunit_suite_checks.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/pyunit/stdlib/resources/pyunit_suite_checks.py b/tests/pyunit/stdlib/resources/pyunit_suite_checks.py index 79944a59fd..4f83294a47 100644 --- a/tests/pyunit/stdlib/resources/pyunit_suite_checks.py +++ b/tests/pyunit/stdlib/resources/pyunit_suite_checks.py @@ -35,6 +35,7 @@ from unittest.mock import patch from gem5.resources.client_api.client_wrapper import ClientWrapper from gem5.resources.resource import ( + ShadowResource, SuiteResource, WorkloadResource, obtain_resource, @@ -112,7 +113,7 @@ class SuiteResourceTestSuite(unittest.TestCase): self.assertIsInstance(filtered_suite, SuiteResource) self.assertEqual(len(filtered_suite), 1) for workload in filtered_suite: - self.assertIsInstance(workload, WorkloadResource) + self.assertIsInstance(workload, ShadowResource) @patch( "gem5.resources.client.clientwrapper", @@ -124,7 +125,7 @@ class SuiteResourceTestSuite(unittest.TestCase): self.assertIsInstance(filtered_suite, SuiteResource) self.assertEqual(len(filtered_suite), 2) for workload in filtered_suite: - self.assertIsInstance(workload, WorkloadResource) + self.assertIsInstance(workload, ShadowResource) @patch( "gem5.resources.client.clientwrapper", From b80a04e1460cf841f37bb10799c80bbbfc6b163d Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Thu, 18 Apr 2024 12:22:17 -0700 Subject: [PATCH 3/5] stdlib,tests: Fix mocked_resquest_post - add kwargs Change-Id: I1c080d42b6f238d2f716c500913dc7576dc13ed6 --- tests/pyunit/stdlib/resources/pyunit_client_wrapper_checks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/pyunit/stdlib/resources/pyunit_client_wrapper_checks.py b/tests/pyunit/stdlib/resources/pyunit_client_wrapper_checks.py index f98005e546..bfcab993a3 100644 --- a/tests/pyunit/stdlib/resources/pyunit_client_wrapper_checks.py +++ b/tests/pyunit/stdlib/resources/pyunit_client_wrapper_checks.py @@ -76,7 +76,7 @@ with open(Path(__file__).parent / "refs/mongo-dup-mock.json") as f: duplicate_mock_json = json.load(f) -def mocked_requests_post(*args): +def mocked_requests_post(*args, **kwargs): # mokcing urllib.request.urlopen class MockResponse: def __init__(self, json_data, status_code): From 52a7218bd8e35e7f0d5568613dfa36948e1db67b Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Thu, 18 Apr 2024 12:24:08 -0700 Subject: [PATCH 4/5] stdlib,tests: Fix test resources entry for to new schema Change-Id: I77c263315d3e7f15df6f7fd83ab4ad9280faf777 --- .../stdlib/resources/refs/suite-checks.json | 20 +++++++++++++++---- .../resources/refs/workload-checks.json | 10 ++++++++-- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/tests/pyunit/stdlib/resources/refs/suite-checks.json b/tests/pyunit/stdlib/resources/refs/suite-checks.json index 7583020292..aa06835a93 100644 --- a/tests/pyunit/stdlib/resources/refs/suite-checks.json +++ b/tests/pyunit/stdlib/resources/refs/suite-checks.json @@ -23,8 +23,14 @@ "description": "Description of workload here", "function": "set_kernel_disk_workload", "resources": { - "kernel": "x86-linux-kernel-5.2.3-example", - "disk-image": "x86-ubuntu-18.04-img-example" + "kernel": { + "id": "x86-linux-kernel-5.2.3-example", + "resource_version": "1.0.0" + }, + "disk-image": { + "id": "x86-ubuntu-18.04-img-example", + "resource_version": "1.0.0" + } }, "additional_params": { "readfile_contents": "echo 'Boot successful'; m5 exit" @@ -40,8 +46,14 @@ "description": "Description of workload here", "function": "set_kernel_disk_workload", "resources": { - "kernel": "x86-linux-kernel-5.2.3-example", - "disk-image": "x86-ubuntu-18.04-img-example" + "kernel": { + "id": "x86-linux-kernel-5.2.3-example", + "resource_version": "1.0.0" + }, + "disk-image": { + "id": "x86-ubuntu-18.04-img-example", + "resource_version": "1.0.0" + } }, "additional_params": { "readfile_contents": "echo 'Boot successful'; m5 exit" diff --git a/tests/pyunit/stdlib/resources/refs/workload-checks.json b/tests/pyunit/stdlib/resources/refs/workload-checks.json index bf954059c5..0daa52269b 100644 --- a/tests/pyunit/stdlib/resources/refs/workload-checks.json +++ b/tests/pyunit/stdlib/resources/refs/workload-checks.json @@ -34,8 +34,14 @@ "description": "Description of workload here", "function": "set_kernel_disk_workload", "resources": { - "kernel": "x86-linux-kernel-5.2.3-example", - "disk-image": "x86-ubuntu-18.04-img-example" + "kernel": { + "id": "x86-linux-kernel-5.2.3-example", + "resource_version": "1.0.0" + }, + "disk-image": { + "id": "x86-ubuntu-18.04-img-example", + "resource_version": "1.0.0" + } }, "additional_params": { "readfile_contents": "echo 'Boot successful'; m5 exit" From 13f85b989fa5c7ed601d09d0f45f1cd65ed273f7 Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Thu, 18 Apr 2024 13:38:32 -0700 Subject: [PATCH 5/5] stdlib: Fix obtaining of Simpoint Resources Change-Id: Ic73547c8c4acbe5d8a30a24dd8709cb2e9f6eb5e --- src/python/gem5/resources/resource.py | 56 +++++++++++++++++++-------- 1 file changed, 40 insertions(+), 16 deletions(-) diff --git a/src/python/gem5/resources/resource.py b/src/python/gem5/resources/resource.py index 591515a6b9..410dd3b018 100644 --- a/src/python/gem5/resources/resource.py +++ b/src/python/gem5/resources/resource.py @@ -481,27 +481,48 @@ class SimpointResource(AbstractResource): self._warmup_interval = warmup_interval self._workload_name = workload_name + self._simpoint_start_insts = None + + def _load_simpoints(self) -> None: + """As we cache downloading of resources until we require it, we may + not pass the simpoint data in the constructor. In this case we enforce + that the simpoint data is loaded via ths `_load_simpoints` function. + Ergo when functions like `get_simpoint_list` are called, the data is + loaded. + """ self._simpoint_start_insts = list( - inst * simpoint_interval for inst in self.get_simpoint_list() + inst * self.get_simpoint_interval() + for inst in self.get_simpoint_list() ) - if self._warmup_interval != 0: + if self.get_warmup_interval() != 0: self._warmup_list = self._set_warmup_list() else: self._warmup_list = [0] * len(self.get_simpoint_start_insts) def get_simpoint_list(self) -> List[int]: """Returns the a list containing all the SimPoints for the workload.""" + if self._simpoint_list is None: + self._load_simpoints() + assert self._simpoint_list is not None, "SimPoint list is None" return self._simpoint_list def get_simpoint_start_insts(self) -> List[int]: """Returns a lst containing all the SimPoint starting instrunction points for the workload. This was calculated by multiplying the SimPoint with the SimPoint interval when it was generated.""" + if self._simpoint_start_insts is None: + self._load_simpoints() + assert ( + self._simpoint_start_insts is not None + ), "SimPoint start insts is None" return self._simpoint_start_insts def get_warmup_interval(self) -> int: """Returns the instruction length of the warmup interval.""" + if self._warmup_interval is None: + self._load_simpoints() + assert self._warmup_interval is not None, "Warmup interval is None" return self._warmup_interval def get_weight_list(self) -> List[float]: @@ -509,6 +530,9 @@ class SimpointResource(AbstractResource): order of the weights matches that of the list returned by ``get_simpoint_list()``. I.e. ``get_weight_list()[3]`` is the weight for SimPoint ``get_simpoint_list()[3]``.""" + if self._weight_list is None: + self._load_simpoints() + assert self._weight_list is not None, "Weight list is None" return self._weight_list def get_simpoint_interval(self) -> int: @@ -520,6 +544,9 @@ class SimpointResource(AbstractResource): Each warmup length in this list corresponds to the SimPoint at the same index in ``get_simpoint_list()``. I.e., ``get_warmup_list()[4]`` is the warmup length for SimPoint ``get_simpoint_list()[4]``.""" + if self._warmup_list is None: + self._load_simpoints() + assert self._warmup_list is not None, "Warmup list is None" return self._warmup_list def get_workload_name(self) -> Optional[str]: @@ -644,20 +671,8 @@ class SimpointDirectoryResource(SimpointResource): self._simpoint_file = simpoint_file self._weight_file = weight_file - # This is a little hack. The functions `get_simpoint_file` and - # `get_weight_file` use the local path, so we set it here despite it - # also being set in the `AbstractResource` constructor. This isn't - # elegant but does not harm. - self._local_path = local_path - ( - simpoint_list, - weight_list, - ) = self._get_weights_and_simpoints_from_file() - super().__init__( simpoint_interval=simpoint_interval, - simpoint_list=simpoint_list, - weight_list=weight_list, warmup_interval=warmup_interval, workload_name=workload_name, local_path=local_path, @@ -668,13 +683,22 @@ class SimpointDirectoryResource(SimpointResource): resource_version=resource_version, ) + def _load_simpoints(self) -> None: + ( + simpoint_list, + weight_list, + ) = self._get_weights_and_simpoints_from_file() + self._simpoint_list = simpoint_list + self._weight_list = weight_list + super()._load_simpoints() + def get_simpoint_file(self) -> Path: """Return the SimPoint File path.""" - return Path(Path(self._local_path) / self._simpoint_file) + return Path(Path(self.get_local_path()) / self._simpoint_file) def get_weight_file(self) -> Path: """Returns the Weight File path.""" - return Path(Path(self._local_path) / self._weight_file) + return Path(Path(self.get_local_path()) / self._weight_file) def _get_weights_and_simpoints_from_file( self,