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

Cannot find action count in newer profiles #113

Closed
saraadams opened this issue Nov 22, 2023 · 1 comment
Closed

Cannot find action count in newer profiles #113

saraadams opened this issue Nov 22, 2023 · 1 comment
Assignees
Labels
type/bug Describes a bug in the codebase.

Comments

@saraadams
Copy link
Collaborator

Description

For more current versions of the Bazel profile, the action counts are not found.
The CLI prints out:
No bottleneck optimizations could be suggested. The Bazel profile does not include an action count, which is required for extracting bottlenecks. Try analyzing a profile that processes actions, for example a build or test.

Observed behavior

No action counts are found.

Expected behavior

Action counts are found.

Step-by-step guide on how to reproduce the bug

  1. Try to analyze a profile generated by a more recent Bazel version, e.g. I was able to reproduce this with 7.0.0.

Additional context

With bazelbuild/bazel@7e92ed3 the action count entries no longer include a tid, which means the action count events are no longer associated with a thread. This breaks our filtering for action counts.

@saraadams saraadams added the type/bug Describes a bug in the codebase. label Nov 22, 2023
saraadams added a commit that referenced this issue Nov 22, 2023
…d with Bazel 7

With Bazel 7, the action count events are no longer part of the `Main Thread`, as they
no longer include a `tid`.
This PR changes the logic, so that the action count can be retrieved both for older style
and newer style profiles.

Contributes to #113.

Signed-off-by: Sara Adams <sara.e.adams@gmail.com>
saraadams added a commit that referenced this issue Nov 22, 2023
…ed with Bazel 7

With Bazel 7, the action count events are no longer part of the `Main Thread`, as they
no longer include a `tid`.
This PR changes the logic, so that the action count can be retrieved both for older style
and newer style profiles.

Contributes to #113.

Signed-off-by: Sara Adams <sara.e.adams@gmail.com>
@saraadams saraadams self-assigned this Nov 24, 2023
copybara-service bot pushed a commit to bazelbuild/bazel that referenced this issue Nov 27, 2023
The JSON trace profile includes counter for data such as action count, CPU usage, memory usage.
The code for writing the events to include these was refactored in a03674e As part of that change the events written no longer include a `tid` (thread ID). However, each event in the JSON trace profile is expected to include a `tid`, as documented in
https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview#heading=h.uxpopqvbjezh

This change ensures the counter series events again have a `tid`, using the current thread's ID. This is consistent with the behavior that was present before the refactor.

In response to #18548 (comment)
Fixes EngFlow/bazel_invocation_analyzer#113

Closes #20300.

PiperOrigin-RevId: 585636103
Change-Id: I2aa1ff1e656324410d120ff5b6071028b0a5dce3
bazel-io pushed a commit to bazel-io/bazel that referenced this issue Nov 27, 2023
The JSON trace profile includes counter for data such as action count, CPU usage, memory usage.
The code for writing the events to include these was refactored in bazelbuild@a03674e As part of that change the events written no longer include a `tid` (thread ID). However, each event in the JSON trace profile is expected to include a `tid`, as documented in
https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview#heading=h.uxpopqvbjezh

This change ensures the counter series events again have a `tid`, using the current thread's ID. This is consistent with the behavior that was present before the refactor.

In response to bazelbuild#18548 (comment)
Fixes EngFlow/bazel_invocation_analyzer#113

Closes bazelbuild#20300.

PiperOrigin-RevId: 585636103
Change-Id: I2aa1ff1e656324410d120ff5b6071028b0a5dce3
@saraadams
Copy link
Collaborator Author

Fixed by bazelbuild/bazel#20300, added to 7.0 via bazelbuild/bazel#20324

keertk pushed a commit to bazelbuild/bazel that referenced this issue Nov 27, 2023
The JSON trace profile includes counter for data such as action count,
CPU usage, memory usage.
The code for writing the events to include these was refactored in
a03674e
As part of that change the events written no longer include a `tid`
(thread ID). However, each event in the JSON trace profile is expected
to include a `tid`, as documented in

https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview#heading=h.uxpopqvbjezh

This change ensures the counter series events again have a `tid`, using
the current thread's ID. This is consistent with the behavior that was
present before the refactor.

In response to
#18548 (comment)
Fixes EngFlow/bazel_invocation_analyzer#113

Closes #20300.

Commit
16acadb

PiperOrigin-RevId: 585636103
Change-Id: I2aa1ff1e656324410d120ff5b6071028b0a5dce3

Co-authored-by: Sara Adams <sara@engflow.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Describes a bug in the codebase.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant