[BazelProfile] Support reading action count of Bazel profiles generated with Bazel 7 #115
Conversation
The functionality provided by `BazelProfile` may need to take the Bazel version into consideration. Therefore, this PR makes the version available within and through the `BazelProfile`, while `BazelVersionDataProvider` stays in place to provide easier access, as well as memoization. Contributes to #109. Signed-off-by: Sara Adams <[email protected]>
…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 <[email protected]>
saraadams
requested review from
lrgoldstein,
restingbull,
TheGrizzlyDev and
WillEngFlow
as code owners
|
Highlighted in bazelbuild/bazel#18548 (comment) |
saraadams
force-pushed
the
sara-move-bazelversion-logic
branch
from
75ab334
to
d60a7ca
Compare
|
If bazelbuild/bazel#20300 is accepted, the |
|
(I also verified that for Bazel 6.4.0 the |
| this.processId = processId; | ||
| this.threadId = threadId; | ||
| } | ||
|
|
||
| public int getThreadId() { | ||
| public Integer getThreadId() { |
There was a problem hiding this comment.
Should this be marked @Nullable?
There was a problem hiding this comment.
I am not planning to merge this PR (hence I converted it to a draft), given that the issue is hopefully resolved on the Bazel side (ensuring profiles do have tid for action count entries) - see bazelbuild/bazel#20300.
|
Closing, as bazelbuild/bazel#20300 was merged and gets rid of the regression. |
With Bazel 7, the action count events are no longer part of the
Main Thread, as they no longer include atid.This PR changes the logic, so that the action count can be retrieved both for older styleand newer style profiles.
Contributes to #113.