mmtk / mmtk-core

Memory Management ToolKit
https://www.mmtk.io
Other
379 stars 69 forks source link

Better support for optional spaces #1173

Open k-sareen opened 4 months ago

k-sareen commented 4 months ago

We should add better support for optional spaces inside plans. Here optional means Option<T> instead of conditional compilation like we have with VM space. Using optional spaces can be useful when you only want to allocate certain spaces when some command-line option has been set, for example.

qinsoon commented 4 months ago

Using optional spaces can be useful when you only want to allocate certain spaces when some command-line option has been set, for example.

Can you give an example when a binding needs to do this (they only know what space to use at run-time, not at build-time)?

If the cost of creating a space is cheap, why do we want to use Option<T> for a space rather than just creating a space and not using it? Do you propose to use Option<T> to replace the current conditional compilation for spaces?

k-sareen commented 4 months ago

So in Android, both the image space and Zygote are command-line options. If one doesn't provide the -Ximage:/path/to/image or -Xzygote options then it will never have an image space or attempt to create a Zygote space.

This is not a big deal for the image space since we can't actually allocate memory for it, but it is relevant to the Zygote space. I originally was implementing it like you said by always having a Zygote space and not using it when the command-line option was not passed, but I thought this would be better software engineering-wise to put the space behind an Option<T>.

If the cost of creating a space is cheap, why do we want to use Option for a space rather than just creating a space and not using it?

Because that requires trusting/ensuring the developer has not accidentally used the space. My stance has always been that something depending on developer discipline does not scale. So in such cases we should leverage Rust's type system to avoid making mistakes.

Do you propose to use Option to replace the current conditional compilation for spaces?

That's an interesting idea and I wasn't considering it (yet), but I think it might actually be cleaner in some ways than our current conditional compilation model.