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

Simulator memory pool per instance #570

Merged
merged 3 commits into from
Jun 18, 2023

Conversation

ohmtech-rdi
Copy link
Owner

@ohmtech-rdi ohmtech-rdi commented Jun 17, 2023

This PR fixes a flaw where the SRAM and SDRAM simulation was entire to the full process, including in the simulator.

This was mainly motivated because operator new is a global function, and passing a memory pool around makes a plug-in porting effort way harder.

This was not an issue in the firmware, as anyway "there is only one module running at a time", but that's not the case for the simulator.
Because the memory pool is static, it is shared between multiple instances of the same module, for example on macOS (this depends on the way dynamic libraries are handled).

This PR makes it so that there is one memory pool per instance of module in the simulator. The implementation keeps a pointer to know which instance is the "current one". Because there can be multiple instances running in different threads, a thread local storage is used for that.
This is ugly, but seems to work.

Todo:

  • Test firmware
  • Test on macOS from Xcode
  • Test on macOS from VS Code
  • Test on macOS from erbb build simulator
  • Test on Windows from erbb build simulator
  • Test on Linux from erbb build simulator

@ohmtech-rdi ohmtech-rdi added the enhancement New feature or request label Jun 17, 2023
@ohmtech-rdi ohmtech-rdi self-assigned this Jun 17, 2023
This will also cleanup the way we implement memory pools by having the
same implementation for the firmware and simulator.
For some reason not really understood, thread_local storage will crash
when arch is set to nocona. Nehalem, which is the one used in vcv rack
sdk, happens to be alright, so we use that.
@ohmtech-rdi ohmtech-rdi force-pushed the simulator-memory-pool-per-instance branch from 68585c7 to 157d457 Compare June 17, 2023 13:13
@ohmtech-rdi
Copy link
Owner Author

ohmtech-rdi commented Jun 18, 2023

We found out that it is not possible to debug on macOS with VS Code, but the problem was at least not introduced in this branch.

@ohmtech-rdi
Copy link
Owner Author

First time we executed erbb install for the firmware, there was no sound output, but turning off and on the module solved it.

@ohmtech-rdi ohmtech-rdi merged commit 697bba7 into main Jun 18, 2023
@ohmtech-rdi ohmtech-rdi deleted the simulator-memory-pool-per-instance branch June 18, 2023 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant