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.

Sign up for GitHub

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

Jump to bottom

Small fixes to perception #594

Merged
merged 1 commit into from
Oct 27, 2023
Merged

Small fixes to perception #594

merged 1 commit into from
Oct 27, 2023

Conversation

qhdwight
Copy link
Collaborator

@qhdwight qhdwight commented Oct 25, 2023 •
edited
Loading

Idk anything about multithreading please check this proper use of mesa monitors

@qhdwight
atomic is not needed here since always modified in critical section, …
576c34b 
…wrong assert that works in release but not debug
@qhdwight qhdwight self-assigned this Oct 26, 2023
ankithu
ankithu reviewed Oct 26, 2023
View reviewed changes
Copy link
Collaborator

@ankithu ankithu left a comment

Choose a reason for hiding this comment

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

where is mIsSwap ready set to true?

@qhdwight
Copy link
Collaborator Author

Right before the mutex unlock. It does not appear in the diff since the assignment operator is overloaded on std::atomic

@ankithu ankithu self-requested a review October 26, 2023 18:50
ankithu
ankithu approved these changes Oct 26, 2023
View reviewed changes
Copy link
Collaborator

@ankithu ankithu left a comment

Choose a reason for hiding this comment

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

ok then yea this LGTM (this is ok since the lock is held while the predicate is being checked so std::atomic was not necessary)

rbridges12
rbridges12 approved these changes Oct 26, 2023
View reviewed changes
Copy link
Collaborator

@rbridges12 rbridges12 left a comment

Choose a reason for hiding this comment

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

LGTMIG

@qhdwight qhdwight merged commit 06ecd14 into master Oct 27, 2023
1 check passed
@qhdwight qhdwight deleted the percep/zed-wrapper-fixes branch October 27, 2023 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rbridges12 rbridges12 rbridges12 approved these changes

@ankithu ankithu ankithu approved these changes

Assignees

@qhdwight qhdwight

Labels
refactor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@qhdwight @ankithu @rbridges12