-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Threading api v3 #1955
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
Threading api v3 #1955
Conversation
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
|
Ok, now that's completely different from the original PR. What about the second part of that PR, |
|
Does anyone know, what these failed windows tests are all about? |
|
Rebase pls |
Support the benchmarking of code, which relies on other multi-threading APIs, e.g. OpenMP.
Support the benchmarking of code, which relies on other multi-threading APIs, e.g. OpenMP.
* Manual thread runner test tests for actual invocation.
9144a2c to
2918a09
Compare
|
I'm not sure what is going on with those failed builds, |
|
Awesome. I will need to sleep on this before doing further review passes, but this does look reasonable. |
|
i'm happy with this but i'll let @LebedevRI make the final decision and merge :) thank you so much @krzikalla |
LebedevRI
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably fine.
Test is rather too bare-bones,
perhaps we should be checking the output?
|
@krzikalla thank you! Any thoughts on the second part of the original PR? |
As requested, here comes a new pull request to enable the user to perform multi-threading benchmarks using other threading APIs.
This one takes another approach, which requires only a few changes in the existing benchmark code.
Personally I like this approach the most, as it decouples the actual benchmark code and the user threading code. In addition, there is no special
thread_stateanymore.Remark: line 305 of
benchmark_runner.cc:manager->WaitForAllThreads();has been removed. I don't think, that this causes any issues, as all threads join directly afterwards (all tests run fine). In fact I think, that some more things regarding threads inbenchmark_runner.cccould be refactored. E.g. I don't understand this code:IMHO the mutex is not needed there, as all spawned threads have already been joined.