Direct ModuleConfig member access

[opensdh]

Would it be possible to have the members of the various ModuleConfig derived classes be public (as some but not all of them are already)?

It was apparently planned to have something beyond the ModuleConfig::set_property interface that would allow clients to directly assign to ModuleConfig members, although I'm not sure what is the significance of "weak linkage" here. I personally would be happy just being able to assign to the members directly (perhaps after downcasting the result of get_module_config), and it would be really nice to be able to construct new objects of these types just to have ordinary value semantics. (This last point is already technically possible.)

[eddy16112]

Why do you want to access the members directly? Could you please provide a use case? The ModuleConfig hides the details of each specific modules, such that applications can only access the allowed members using set/get functions.

[opensdh]

I'd like (my) users to be able to write

cuda_cfg.cfg_fb_mem_size = 512 << 20;

rather than

cuda_cfg.set_property("fb_mem", 512 << 20);

which contains two if not three bugs.

[opensdh]

I must also point out that the current types do not behave properly with copying:

auto &cfg = static_cast<…ModuleConfig&>(*Realm::Runtime::get_runtime().get_module_config("…"));
auto copy = cfg;
copy.set_property("…", …);

modifies cfg but not copy because the config_map copy still points into the original. ModuleConfig also fails to be copy-assignable on some implementations because of the void *const element type. Finally, moving it breaks set_property in the original (beyond the usual "valid but unspecified state" idea, I would say) because it leaves config_map empty.

Obviously all these things can be avoided by making the types immovable, but I would argue instead that they suggest that the various restrictions on using these types are artificial and unhelpful. Making the types simpler and using a static map to (reinterpret_casted) pointers to members (which could still be selected via a virtual function) would achieve much the same thing without those restrictions.

[eddy16112]

Making the types simpler and using a static map to (reinterpret_casted) pointers to members (which could still be selected via a virtual function) would achieve much the same thing without those restrictions.

Yeah, I think that is a good idea.

cuda_cfg.cfg_fb_mem_size = 512 << 20;

This requires weak linkage or protect the code with REALM_USE_CUDA. I can not remember why we designed the string based set/get in the first place, maybe we would like to get rid of REALM_USE_XYZ。

[opensdh]

Sorry if I'm missing something obvious, but I'm still not sure which weak-linkage issues you have in mind. Obviously if Realm defines CudaModuleConfig only in some configurations, use of it has to be similarly restricted, but that header specifically mentions that it always exists as a type. The module itself might not exist, which could be handled either by not trying to copy a configuration out of it or (if we could construct these directly) by not trying to copy a configuration into it.

(Also, I'm no longer sure that std::unordered_map and friends even allow a const-qualified mapped type, but that won't matter as much if it's a static object anyway.)

[eddy16112]

The CudaModuleConfig only exists if REALM_USE_CUDA is enabled, but https://github.com/StanfordLegion/realm/blob/main/tests/machine_config_test.cc#L574-L615 this example works (we could query the availability of the cuda module) even if the USE_CUDA is disabled.

[lightsighter]

Catching back up here...

I can not remember why we designed the string based set/get in the first place, maybe we would like to get rid of REALM_USE_XYZ

If I remember correctly we didn't want users to have to rebuild against a different Realm interface just because a machine may or may not have a particular module (e.g. the CUDA module). With the current interface you can compile any object against a common librealm interface. Then dynamically when code starts running, it can dynamically introspect Realm to see which modules exist (e.g. am I running on a machine with a CUDA module or not) and then if you find such a module you can configure it appropriately. It means you don't have to compile two versions of your client code: one for machines with CUDA and one for machines without it. Doing so would be really annoying because you'd either have to put macros in your own code to mask CUDA code for the non-CUDA build or build on a machine that has CUDA (and all its headers installed) even though you're ultimately going to run on a machine without CUDA. So the tradeoff is that this interface is portable and makes users' lives "easier", but it also gives up all the static checking of things like types and variable names that you could get if you encoded the actual names and types of configuration variables in structs that the user can mutate directly.

That's the trade-off that is being made. If I recall it was @muraj that was advocating for the dynamic approach and maybe he has some additional thoughts on why he liked the dynamic interface better.