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

Miscellaneous emscripten fixes #510

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

topolarity
Copy link
Contributor

Series of emscripten-specific fixes:

  • Increase STACK_SIZE to 128 kB from 64 kB (was causing crashes, especially on startup)
  • Re-scale horizontal scroll values so that scroll speed (approximately) matches desktop
  • Fix NULL pointer dereference
  • Increase PTHREAD_POOL_SIZE from 4 to 8 (may be unnecessary)

Emscripten's stack size was recently decreased to 64 kB from 5 MB,
(emscripten-core/emscripten#18191).

Stack overflow appears to be the cause of frequent crashes of Tracy
in my browser, especially at start-up. This increase is modest, but
seems to be enough to resolve the issue.
In my browser, I noticed that zooming seemed jumpy and unreliable.
Turns out that my fingers were moving horizontally on my trackpad,
and the x-scroll was causing the unexpected behavior.
I never noticed this causing any user-visible deadlocks, but I saw
warnings in the Javascript console that the thread pool was getting
exhausted. Increasing it to this number seems to resolve the issue.
@wolfpld
Copy link
Owner

wolfpld commented Jan 18, 2023

If the pthread pool size change may be unnecessary, why is it made?

@wolfpld
Copy link
Owner

wolfpld commented Jan 18, 2023

imgui_impl_glfw.cpp is sourced from ImGui. Please submit your changes to ImGui first. I don't want to maintain yet another patch on foreign sources.

@topolarity
Copy link
Contributor Author

If the pthread pool size change may be unnecessary, why is it made?

The thread pool is definitely being exhausted, but I don't know enough about Tracy's usage of threads to know when it depends on being able to execute all its threads concurrently.

Happy to remove the change if you'd prefer.

imgui_impl_glfw.cpp is sourced from ImGui. Please submit your changes to ImGui first. I don't want to maintain yet another patch on foreign sources.

Fair enough - I've submitted ocornut/imgui#6096

@wolfpld
Copy link
Owner

wolfpld commented Jan 18, 2023

The thread pool is definitely being exhausted, but I don't know enough about Tracy's usage of threads to know when it depends on being able to execute all its threads concurrently.

Happy to remove the change if you'd prefer.

I have seen some weird problems related to threads, but these were mainly on Chrome-based browsers. Firefox seems to handle this much better.

There are already some ifdefs to minimize thread usage with emscripten, but maybe this is not enough?

https://github.com/wolfpld/tracy/blob/e4bd88c/server/TracyWorker.cpp#L1271-L1277

@topolarity
Copy link
Contributor Author

Good point. I'll try to investigate this weekend and see I can track down where the threads are being started

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