Deemphasize setting options by strings

[wks]

Currently there are too many ways to set MMTk options by string.

• memory_manager::process
• memory_manager::process_bulk
• MMTKBuilder::set_option
• MMTKBuilder::set_options_bulk_by_str
• Options::set_from_env_var [1]
• Options::set_from_command_line [1]
• Options::set_bulk_from_command_line

Note [1]: After #1237 is resolved, there will not be difference between setting from environment variables or command line arguments.

This is harmful in two ways.

Firstly, it is repetitive. Ideally there should be one obvious way to do it.

Secondly, they distract the users from the most direct and safest way of setting options, i.e. calling MMTKOption::set directly. Those methods, particularly those in memory_manager, makes users think options can only be set by strings. For example, in mmtk/mmtk-openjdk#229 (comment), a developer is unaware that MMTKBuilder::options can be modified directly, and therefore attempted to set options by first converting numbers to strings and then using Builder::set_option to parse the string.

What should we do?

Remove aliases

We should remove the methods in memory_manager and MMTKBuilder which do the same thing as the methods in Options. This will force the users to access the Options type when they want to set options.

Emphasize that setting options by string is not the best way

Naming the method

We should rename Options::set_from_env_var and Options::set_from_command_line to Options::set_by_string (which #1240 already does). This sends a clear message that this method sets the option by string which involves parsing, and is not the default way to set options. That PR also changed the documentation to emphasize this.

(optional) Introduce get_dynamic_option

We may also replace it with Options::get_dynamic_option(name: &str) -> &mut dyn MMTKOptionDyn, where

trait MMTKOptionDyn {
    fn set_by_string(&mut self, value_str: &str) -> Result<(), SetByStringError>;
}

impl<T> MMTKOptionDyn for MMTKOption<T> {
    fn set_by_string(&mut self, value_str: &str) -> Result<(), SetByStringError> {
        let value = value_str.parse()?;  // `T` is automatically inferred
        self.set(value)?;
        Ok(())
    }
}

This trait does not have any type argument (unlike MMTKOption<T>), and can only be used to set the option by string. It also sends a message that the users only need to use this method if they don't know the option name at compile time (therefore look up the option by name). The value always implements FromStr (which #1240 also fixes) so that the user can always call, for example, options.threads.set(user_input.parse()), if they know the option key threads at compile time.

This may be two complicated. If a well-named method set_by_string is sufficient, we can just use that.

[k-sareen]

Uhh yes and no. Currently options are immutable, i.e. they don't allow you to change them after MMTk creation. But this may change in the future (and in fact, it'd be nice to have this if possible for things like "full_heap_system_gc" etc.). MMTKBuilder is only useful at creation time.