Reading a property value in an immutable context

Notes on different solutions to the problem of reading a property value in an immutable context

[These notes were outdated before they were posted. But I'm putting them here temporarily until I flesh out something more complete and representative.]

Bypass any code paths that potentially mutate context during a property value read

Ultimately this boils down to the case of a derived property; in all other cases we can directly access the underlying property value store ourselves

Adding a PersonProperty::compute_immutable() method

Property's could have a separate fallible compute method, compute_immutable:

/// Compute the value of the property, possibly by accessing the context and using the person's ID.
/// It is possible that this method mutates `context`, for example, when reading the property value
/// for the first time also _writes_ the value to the backing store.
fn compute(context: &Context, person_id: PersonId) -> Self::Value;

/// Used to compute the value in a context in which mutation of `context` is prohibited. This method should panic
/// if it is unable to compute the value without mutation.
fn compute_immutable(context: &Context, person_id: PersonId) -> Self::Value;

In this design, the fact that the "immutable phase" of a query will always succeed is explicit: We are explicitly calling the compute_immutable method and are guaranteed (well, via a pinky promise—see below) to not encounter a "double borrow" error because of the explicit API contract.

Whether or not the results of the query are semantically correct does require that the "mutable phase" be performed correctly, but again, this is explicit in the contract of the API: The "mutable phase" is performed right before the query is run, and thus we know that all mutations that are theoretically required in order to just read a value have been done.

(Maybe it returns a Result instead?)

Pros:

• conceptually straightforward, easy to read and understand.
• Implementation is straightforward
• Implementation inside macros means client code isn't required to "get it right"—the macro gets it right on behalf of the client code.

Cons:

• No enforcement at level of language, e.g. by the type system. It's just a pinky promise to do the right thing.
• The implementor should understand the point of having this, namely that we want to separate mutation from immutable reads in some contexts (e.g. queries).
• Relies on correct implementation within macros.
• Expands API for edge cases related to internal implementation details.
• It's just plain awkward to have two different ways do something as basic as "fetch the value for this property for this person."

Modify PersonProperty::compute() to have a separate code path if mutation is required

We could refactor the implementation of PersonProperty::compute() to be something like this:

fn compute(context: &Context, person_id: PersonId) -> Self::Value {
  // Try to compute the value using only an immutable reference.
  {
    // The scope of the immutable borrow(s)
    let data_container = context.get_data(PeopleData);
    // Immutable borrow of `RefCell`
    let property_map = data_container.property_map.borrow();
    if let Some(property_vec) = property_map.get(Self::type_id()) {
      // ...
      // This might fail to fetch / compute the value, either because the 
      // property vector doesn't yet exist, or because the values in the
      // property vector haven't been initialized....
      // ...
      if (value_successfully_computed) {
        return successfully_computed_value;
      }
    }
  }
  
  // If we get to this point, we failed to compute the value in an immutable contex.
  {
    // The scope of the MUTABLE borrow(s)
    let mut properties_map = self.properties_map.borrow_mut();

    // Fetch existing or initialize a new property store
    match properties_map.entry(Self::type_id()) {
        Entry::Occupied(entry) => {
            // exists, but value for this particular person ID needs to be initialized
        }

        Entry::Vacant(entry) => {
            // needs to be created
        },
    };
    // ..
  }
}

(The explicit scope blocks are just illustrative, not actually technically necessary.)

In this design, the fact that the "immutable phase" of a query will always succeed is implicit: we know it will succeed because we know we do the "mutable phase" right before the query is run, and thus we know that all mutations that are theoretically required in order to just read a value have been done.

Pros:

• Don't need to create a new method, no need to expand API.
• Only pay the price of the mutable borrow when we need to.
• Macro implementations make sure client code does the right thing—client code doesn't need to "get it right."

Cons:

• Is just a pinky promise to do the right thing. The fact that it does the right thing relies on macro implementation being correct (which we presumably have control over).
• The "slow path" is annoyingly redundant: We borrow something, try to read a value and, if reading fails, drop the borrow and borrow again (this time mutably). (This is an aesthetic objection, not a technical one. Performance implications would need to be measured.)
• Requires to leak private implementation of PeopleData and properties not just outside of the People module, but outside of ixa itself. (PersonProperty types are defined external to the ixa crate but would need to interact with private implementation.)
  • We could mitigate this somewhat by added methods to ContextPeopleExt to service these needs, but that expands the public API for reasons related to underlying private implementation.
• Possible performance implications for what might be a common case, namely the case that reading a value also mutates context. However, the performance implications are not at all clear and needs to be measured.