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

Add multithreading to libFLAC #634

Merged
merged 24 commits into from
Sep 22, 2023
Merged

Add multithreading to libFLAC #634

merged 24 commits into from
Sep 22, 2023

Conversation

ktmf01
Copy link
Collaborator

@ktmf01 ktmf01 commented Jul 6, 2023

No description provided.

@ktmf01 ktmf01 changed the title Prepare for crude attempt at multithreading Add multithreading to libFLAC Jul 7, 2023
@ktmf01 ktmf01 force-pushed the pthread2 branch 3 times, most recently from 6675c2a to 7ff207f Compare July 10, 2023 19:31
@ktmf01 ktmf01 force-pushed the pthread2 branch 2 times, most recently from eb9a588 to ccd6af6 Compare July 11, 2023 06:45
This scales the number of active threads back when threads have to
wait for work too often
@ktmf01 ktmf01 force-pushed the pthread2 branch 2 times, most recently from ca291d7 to fa59edd Compare August 7, 2023 12:33
@ktmf01 ktmf01 marked this pull request as ready for review August 7, 2023 17:51
@ktmf01
Copy link
Collaborator Author

ktmf01 commented Aug 12, 2023

A lively discussion with test results, comparisons and suggestions can be found here: https://hydrogenaud.io/index.php/topic,124437.0.html

@ktmf01 ktmf01 merged commit 8cf7e7f into xiph:master Sep 22, 2023
14 checks passed
@sezero
Copy link
Contributor

sezero commented Sep 23, 2023

This seems to only implement threading via pthreads even for windows, e.g. autotools/cmake check pthreads unconditionally. Is that really the intention? (I know mingw-w64 has pthread implementation, but still...)

@ktmf01
Copy link
Collaborator Author

ktmf01 commented Sep 24, 2023

Yes. This has been extensively tested for Windows on mingw-w64. Could you explain what you think is wrong with that?

@sezero
Copy link
Contributor

sezero commented Sep 24, 2023

Could you explain what you think is wrong with that?

The extra dependency on the pthread dll whereas win32 apis could be used directly. But that's an administrative decision, so I won't fight it.

@ktmf01
Copy link
Collaborator Author

ktmf01 commented Sep 24, 2023

I don't know what you mean with administrative. I would think to use the win32 api's directly would mean adding quite a bit of code, because those APIs can't be used in the same way pthreads can be used? That code would get little CI coverage and no fuzzing coverage, so I'd rather not do that.

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.

2 participants