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

Optionally store StaticPyObjStore on micropython heap via root pointer #20

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

Conversation

swuerl
Copy link

@swuerl swuerl commented Nov 29, 2024

Related to #16

This optionally moves the StaticPyObjStore from being attached to a random module to a new global storage for micropython-wrap that uses a micropython root pointer. The root pointers are saved from garbage collection in micropythons internal implementation. As such the implementations should be equal.

In the process a MPyMapView class was added, which allows for C++-style access to micropython maps.

IMHO this should be default behaviour in the future: this makes the _StaticPyObjStore variable, which is an implementation detail of micropython-wrap, invisible to the user. I did not enable it by default here, as it breaks the interface by requiring explicit definition of the root pointer by the user. Once we have some confidence with this implementation, we could maybe add a deprecation #warning in case it is not enabled? But maybe you also just want to keep the old behaviour.

@swuerl
Copy link
Author

swuerl commented Nov 29, 2024

Can you maybe help me out here? https://ci.appveyor.com/project/stinos/micropython-wrap/builds/51082056/job/tjk2dtc0su3ch6pr

It seems the root pointer is not correctly registered for the windows build. In the code, this happens in tests/cmodule.c.

Is cmodule.c not used in every case? Or is this a caching thing? This works on unix

@stinos
Copy link
Owner

stinos commented Dec 3, 2024

Is cmodule.c not used in every case?

Indeed it is not, it is used only for those builds which use a user C module. So that's essentially never for the msvc builds, and only for 1 from the 3 build types in the Makefile

@swuerl
Copy link
Author

swuerl commented Dec 3, 2024

Ah, understood. In this case that means micropython needs to be aware of the root-pointer for micropython-wrap, even if the modules themselfes are dynamically linked in later.

For the unix/msys2 builds this should be possible by injecting it via the QSTR_GLOBAL_DEPENDENCIES make variable.

Do you know if there is something similar possible for the msvc build?

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