Skip to content

Conversation

@williamjallen
Copy link
Collaborator

@williamjallen williamjallen commented Dec 3, 2024

This PR replaces the initial build instrumentation work in #2460 with a more complete version based on a working CMake/CTest implementation. See #2395 for the full feature request.

I plan to make a follow-up PR to add support for configure measurements once a handful of blocking database schema changes are completed. Future PRs will also implement various UIs to display the data collected here.

@williamjallen williamjallen added this to the v3.7 milestone Dec 3, 2024
@williamjallen williamjallen marked this pull request as draft December 3, 2024 21:26
@williamjallen williamjallen changed the title Add build instrumentation handling logic Add build instrumentation API Dec 15, 2024
@williamjallen williamjallen force-pushed the build-instrumentation branch 2 times, most recently from a4106b9 to 9128d2d Compare December 17, 2024 15:38
@williamjallen williamjallen modified the milestones: v3.7, v3.8 Jan 2, 2025
@williamjallen williamjallen force-pushed the build-instrumentation branch 2 times, most recently from ee592af to 9811123 Compare January 15, 2025 13:21
@williamjallen williamjallen modified the milestones: v3.8, v3.9 Jan 28, 2025
github-merge-queue bot pushed a commit that referenced this pull request Feb 17, 2025
This PR pulls out some of the logic in
#2612 for matching full XML paths
into a separate PR in anticipation of several upcoming PRs. By
specifying the full XML path for an element, the exact element being
targeted is more clear. For the sake of example, I used the new path
method to locate all of the `<Site>` elements.
@williamjallen williamjallen force-pushed the build-instrumentation branch 6 times, most recently from 6fe8a11 to 02f18e7 Compare February 28, 2025 16:02
@williamjallen williamjallen modified the milestones: v3.9, v4.0 Feb 28, 2025
@williamjallen williamjallen force-pushed the build-instrumentation branch 2 times, most recently from 78a147b to 0d9f57f Compare March 2, 2025 01:26
@williamjallen williamjallen marked this pull request as ready for review March 2, 2025 02:11
Copy link
Member

@josephsnyder josephsnyder left a comment

Choose a reason for hiding this comment

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

LGTM! once the data was properly added into the database, it made sense and was able to be pulled out in GraphQL. As discussed F2F, I'm not a big fan of command and commands being attributes on the build but I'm not willing to let that stop this from moving forward

@williamjallen williamjallen force-pushed the build-instrumentation branch from 0d9f57f to e7d81a8 Compare March 21, 2025 17:05
@williamjallen williamjallen force-pushed the build-instrumentation branch 3 times, most recently from d02443c to 84312b9 Compare March 28, 2025 18:04
Copy link
Member

@josephsnyder josephsnyder left a comment

Choose a reason for hiding this comment

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

Thanks for the quick help, Everything here LGTM

@williamjallen williamjallen force-pushed the build-instrumentation branch 2 times, most recently from a95ca23 to 210697a Compare April 7, 2025 16:50
@williamjallen williamjallen force-pushed the build-instrumentation branch from 210697a to 18a350e Compare April 10, 2025 19:54
@williamjallen williamjallen force-pushed the build-instrumentation branch from 1764b27 to 91cf21f Compare April 22, 2025 11:56
@williamjallen williamjallen added this pull request to the merge queue Apr 22, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 22, 2025
@williamjallen williamjallen added this pull request to the merge queue Apr 22, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 22, 2025
@williamjallen williamjallen added this pull request to the merge queue Apr 22, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 22, 2025
@williamjallen williamjallen added this pull request to the merge queue Apr 22, 2025
Merged via the queue into Kitware:master with commit 989e047 Apr 22, 2025
13 of 14 checks passed
@williamjallen williamjallen deleted the build-instrumentation branch April 22, 2025 17:09
github-merge-queue bot pushed a commit that referenced this pull request May 2, 2025
Following the work started in
#2612, this PR adds supports for
the `output` and `outputSizes` [snippet
file](https://cmake.org/cmake/help/latest/manual/cmake-instrumentation.7.html#v1-snippet-file)
fields.
github-merge-queue bot pushed a commit that referenced this pull request May 2, 2025
Following the work started in
#2612, this PR adds supports for
the `output` and `outputSizes` [snippet
file](https://cmake.org/cmake/help/latest/manual/cmake-instrumentation.7.html#v1-snippet-file)
fields.
github-merge-queue bot pushed a commit that referenced this pull request Jul 28, 2025
Building on top of #2612, this PR
adds a new "build targets" page which displays a table of targets for a
build if instrumentation data was submitted. See
#2827 for more details about the
outline for our new instrumentation-related pages. I have a number of
future improvements in mind for this page, including better support when
viewing parent builds, correlation to errors & warnings with associated
color highlighting, customizable timing or other measurement columns, a
trace-style view of the targets or other at the top of the page.

<img width="2834" height="944" alt="image"
src="https://github.com/user-attachments/assets/1a84ad9d-1197-4c38-a696-8c935c3116ef"
/>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants