Skip to content

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Oct 17, 2025

Just like for other types of symbols we don't want to export symbols on the Module object itself if they user requested this.

Also, fix the memory profiler to not depend the external export of __heap_base but instead the internal version (like it does for _sbrk and other dependencies.

Just like for other types of symbols we don't want to export symbols
on the `Module` object itself if they user requested this.

Also, fix the memory profiler to not depend the external export of
`__heap_base` but instead the internal version (like it does for
`_sbrk` and other dependencies.
@sbc100 sbc100 requested review from dschuff and juj October 17, 2025 18:02
@sbc100 sbc100 enabled auto-merge (squash) October 17, 2025 18:03
Copy link
Member

@dschuff dschuff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this should have a changelog entry too, since it might be user visible?

@sbc100
Copy link
Collaborator Author

sbc100 commented Oct 17, 2025

Maybe this should have a changelog entry too, since it might be user visible?

Im not actually sure this will be user visible... or at least I'm having trouble articulating exactly how it would be.

The thing is that the users visible controls are not effected e.g EMSCRIPTEN_KEEPALIVE and EXPORTED_FUNCTIONS still continue to work as expected and export the symbols.

Its native symbols that were added to settings.REQUIRED_EXPORTS internally that are no longer exported, and this is not user-visible setting.

@sbc100
Copy link
Collaborator Author

sbc100 commented Oct 17, 2025

I could say something like:

"Data symbols that were exported from the wasm module for internal use are no longer exports on the Module object."?

I don't know how common this is outside of this specific --memory-profiler usage though.

@juj
Copy link
Collaborator

juj commented Oct 17, 2025

Im not actually sure this will be user visible...

The output on the Module object will be user visible. Though whether anyone depended on data in there, it is harder to say.

LGTM, a note in ChangeLog might be good here.

@sbc100 sbc100 merged commit 30c6c4d into emscripten-core:main Oct 17, 2025
34 checks passed
@sbc100 sbc100 deleted the memprof_cleanup branch October 17, 2025 18:59
@sbc100
Copy link
Collaborator Author

sbc100 commented Oct 17, 2025

Im not actually sure this will be user visible...

The output on the Module object will be user visible. Though whether anyone depended on data in there, it is harder to say.

LGTM, a note in ChangeLog might be good here.

Note the requests to export data symbols like via EMSCRIPTEN_KEEPALIVE and/or EXPORTED_FUNCTIONS are not effect by this change. Those once we still continue to export.

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.

3 participants