- Notifications
You must be signed in to change notification settings - Fork3.5k
Conditionally compile the pthread_t member in emscripten::val#25872
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
val adds a `pthread_t` data member for later thread checks, but it is onlyused when the code is compiled with pthread support.Following the "what you don't use, you don't pay for" principle, thischange wraps the `pthread_t` member with the same `#ifdef` guard used bythe thread-checking code. For single-threaded builds, the member is omittedentirely, thus avoiding unnecessary memory overhead.
xbcnn commentedNov 26, 2025
@RReverser@sbc100 Please help review. Thanks. |
sbc100 commentedNov 26, 2025
It the extra word really a problem? If it is significant should we also put this behind |
xbcnn commentedNov 27, 2025
I think it is not significant if not widely used. However, when you have thousands of them, the cumulative memory overhead can become noticeable. Guarding with |
RReverser commentedNov 27, 2025
I remember in the original PR where I added this field I was also initially guarding this field with a compilation guard, but@sbc100 suggested the extra complexity is not worth it as the size is fairly small. I'm fine either way. |
sbc100 commentedNov 27, 2025
I think only risk here is that different compilation units will end up with different stuct sizes.. which could be very bad |
RReverser commentedNov 27, 2025
Oh right, I remember you mentioning that. |
xbcnn commentedNov 27, 2025
I didn't notice that when I made this PR.
Normally this won't happen if entire application is built from source. But for prebuilt libs, there could be subtle bugs.., if |
RReverser commentedNov 27, 2025
Even for building from source, you can build some objects / libraries with When you link them together (mix & match is allowed), it will result in incompatible struct layouts, which is going to lead to memory corruptions that will be rather hard to debug. |
xbcnn commentedNov 28, 2025
Yeah, even it is not common engineering practices. |
val adds a
pthread_tdata member for later thread checks, but it is only used when the code is compiled with pthread support.Following the "what you don't use, you don't pay for" principle, this change wraps the
pthread_tmember with the same#ifdefguard used by the thread-checking code. For single-threaded builds, the member is omitted entirely, thus avoiding unnecessary memory overhead.