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

Ensure that thread-shared variable is initialized by single thread #2373

Merged
merged 6 commits into from
Aug 22, 2022

Conversation

heplesser
Copy link
Contributor

@heplesser heplesser commented Apr 20, 2022

This PR ensures that the thread-global variable decrease_buffer_size_spike_data_ is initialized by a single thread before entering the region in which it is used.

@heplesser heplesser added S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation. I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) labels Apr 20, 2022
@heplesser heplesser requested review from jougs and suku248 April 20, 2022 13:05
@heplesser heplesser added this to PRs in progress in Kernel via automation Apr 20, 2022
@jougs
Copy link
Contributor

jougs commented May 13, 2022

@heplesser: I am generally positive towards the changes, but can you please provide a description of the problem this is solving or a link to an issue?

@heplesser
Copy link
Contributor Author

@heplesser: I am generally positive towards the changes, but can you please provide a description of the problem this is solving or a link to an issue?

I have added a short description and merged master so the actual change becomes better visible.

Kernel automation moved this from PRs in progress to PRs approved May 23, 2022
Copy link
Contributor

@jougs jougs left a comment

Choose a reason for hiding this comment

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

Thanks!

@github-actions
Copy link

Pull request automatically marked stale!

@github-actions github-actions bot added the stale Automatic marker for inactivity, please have another look here label Jul 22, 2022
Copy link
Contributor

@suku248 suku248 left a comment

Choose a reason for hiding this comment

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

Thanks @heplesser !

Just for the record: This is one of the suspicious lines of code we checked during one of our Hackathons?
And we concluded that:

  • not having this in a single environment did not result in erroneous behavior, but it has the potential to cause errors in thread-parallel code, and
  • runtime measurements showed that the single environment does have an effect on the runtime?

@heplesser
Copy link
Contributor Author

Just for the record: This is one of the suspicious lines of code we checked during one of our Hackathons? And we concluded that:

  • not having this in a single environment did not result in erroneous behavior, but it has the potential to cause errors in thread-parallel code, and
  • runtime measurements showed that the single environment does have an effect on the runtime?

Yes to all!

@heplesser heplesser merged commit 2ba3ad2 into nest:master Aug 22, 2022
Kernel automation moved this from PRs approved to Done (PRs and issues) Aug 22, 2022
@heplesser heplesser deleted the add_single branch October 4, 2022 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: Normal Handle this with default priority stale Automatic marker for inactivity, please have another look here T: Maintenance Work to keep up the quality of the code and documentation.
Projects
Kernel
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants