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 GC thread in newer profiles #110

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

Cannot find GC thread in newer profiles #110

saraadams opened this issue Nov 21, 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 garbage collector thread is not found.
The CLI prints out:
This does not appear to be a valid Bazel profile. Unable to find garbage collector thread.

Observed behavior

The GC thread is not found.

Expected behavior

The GC thread is found.

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

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

Additional context

It seems the GC events are now in a thread named "Notification Thread".
We could update the code to look for that thread - or ignore the thread name and simply look for entries that have the category gc notification. The latter might be better, as I have also seen valid Bazel profiles that include no GC / notification thread.

@saraadams saraadams added the type/bug Describes a bug in the codebase. label Nov 21, 2023
saraadams added a commit that referenced this issue Nov 22, 2023
The thread in which GC events are listed has been renamed multiple
times, so the code for extracting the right thread by name is
error-prone.
Instead of testing the name, search for the GC thread by its included
events. This assumes that there is only one thread that contains GC events.

Contributes to #110

Signed-off-by: Sara Adams <sara.e.adams@gmail.com>
saraadams added a commit that referenced this issue Nov 22, 2023
Currently, a Bazel profile with no GC threads is identified as
invalid. However, in some cases valid Bazel profiles may
not include any garbace collection events.
With this PR we no longer throw an error if no GC is found, but
rather emit a warning that no GC analysis could be performed.

Contributes ton #110.

Signed-off-by: Sara Adams <sara.e.adams@gmail.com>
saraadams added a commit that referenced this issue Nov 23, 2023
The thread in which GC events are listed has been renamed multiple
times, so the code for extracting the right thread by name is
error-prone.
Instead of testing the name, search for the GC thread by its included
events. This assumes that there is only one thread that contains GC
events.

Contributes to #110

---------

Signed-off-by: Sara Adams <sara.e.adams@gmail.com>
saraadams added a commit that referenced this issue Nov 23, 2023
Currently, a Bazel profile with no GC threads is identified as
invalid. However, in some cases valid Bazel profiles may
not include any garbace collection events.
With this PR we no longer throw an error if no GC is found, but
rather emit a warning that no GC analysis could be performed.

Contributes ton #110.

Signed-off-by: Sara Adams <sara.e.adams@gmail.com>
@saraadams saraadams self-assigned this Nov 24, 2023
saraadams added a commit that referenced this issue Nov 24, 2023
Currently, a Bazel profile with no GC threads is identified as invalid.
However, in some cases valid Bazel profiles may not include any garbace
collection events.
With this PR we no longer throw an error if no GC is found, but rather
emit a warning that no GC analysis could be performed.

Contributes ton #110.

Signed-off-by: Sara Adams <sara.e.adams@gmail.com>
copybara-service bot pushed a commit to bazelbuild/bazel that referenced this issue Nov 27, 2023
Currently, the thread that includes the garbage collection notification events is not correctly identified.
This means that in the JSON trace profile, the thread:

* is not renamed, as desired.
* is not sorted towards the top, as desired.

The thread is identified by its name, which has changed from "Service Thread" to "Notification Thread". This change updates the profile code accordingly.

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

Closes #20299.

PiperOrigin-RevId: 585639069
Change-Id: Id4aadb5451839b5be07ac98428f9367764ff46ce
bazel-io pushed a commit to bazel-io/bazel that referenced this issue Nov 27, 2023
Currently, the thread that includes the garbage collection notification events is not correctly identified.
This means that in the JSON trace profile, the thread:

* is not renamed, as desired.
* is not sorted towards the top, as desired.

The thread is identified by its name, which has changed from "Service Thread" to "Notification Thread". This change updates the profile code accordingly.

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

Closes bazelbuild#20299.

PiperOrigin-RevId: 585639069
Change-Id: Id4aadb5451839b5be07ac98428f9367764ff46ce
@saraadams
Copy link
Collaborator Author

Fixed by bazelbuild/bazel#20299, added to 7.0 by bazelbuild/bazel#20327

keertk pushed a commit to bazelbuild/bazel that referenced this issue Nov 27, 2023
Currently, the thread that includes the garbage collection notification
events is not correctly identified.
This means that in the JSON trace profile, the thread:

* is not renamed, as desired.
* is not sorted towards the top, as desired.

The thread is identified by its name, which has changed from "Service
Thread" to "Notification Thread". This change updates the profile code
accordingly.

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

Closes #20299.

Commit
13829ed

PiperOrigin-RevId: 585639069
Change-Id: Id4aadb5451839b5be07ac98428f9367764ff46ce

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