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

Remove thread state change callstack feature flag #4256

Conversation

florian-kuebler
Copy link
Collaborator

This removes the feature flag for callstacks on thread state changes.

Test: Local
Bug: http://b/235554760

This removes the feature flag for callstacks on thread state
changes.

Test: Local
Bug: http://b/235554760
}

uint16_t stack_dump_size = static_cast<uint16_t>(
Copy link
Collaborator

Choose a reason for hiding this comment

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

thread_state_change_stack_dump_size to not confuse with the one for sampling.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@karupayun karupayun self-assigned this Oct 4, 2022
Copy link
Collaborator

@karupayun karupayun left a comment

Choose a reason for hiding this comment

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

LGTM! :)

.value(kThreadStateChangeCallstackMaxCopyRawStackSizeSettingKey,
orbit_qt::CaptureOptionsDialog::kThreadStateChangeMaxCopyRawStackSizeDefaultValue)
.toUInt());
app_->SetThreadStateChangeCallstackStackDumpSize(stack_dump_size);
Copy link
Collaborator

Choose a reason for hiding this comment

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

We were setting the stack_dump_size as max before in case of unspecified callstack collection. I guess that this doesn't matter at all in this case, since it will be ignored, but could we have a potential issue because unespecified value?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, that shouldn't be an issue. That value is ignored in that case anyways.

@florian-kuebler florian-kuebler merged commit b8ddcc7 into google:main Oct 4, 2022
@florian-kuebler florian-kuebler deleted the feature/release_thread_state_change_callstacks branch October 4, 2022 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants