Skip to content

Miscellaneous emscripten fixes#510

Open
topolarity wants to merge 4 commits intowolfpld:masterfrom
topolarity:emscripten-fixes
Open

Miscellaneous emscripten fixes#510
topolarity wants to merge 4 commits intowolfpld:masterfrom
topolarity:emscripten-fixes

Conversation

@topolarity
Copy link
Copy Markdown
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
Copy Markdown
Owner

wolfpld commented Jan 18, 2023

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

@wolfpld
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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