Skip to content

Add initial draft of static immutability #126

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 6, 2018
Merged

Conversation

leafpetersen
Copy link
Member

cc @aam @mraleph @munificent @lrhn for comment.

@zoechi
Copy link

zoechi commented Dec 5, 2018

class Value<T> extends Scalar<T> implements Constant is immutable
  where T is immutable {
}

would this support if(Value<String>() is immutable) {...} checks like the "Alternative syntax"?

Question: Do we need additional syntax for the case where a static type context
is not required?

```
Copy link
Member

Choose a reason for hiding this comment

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

Yes, please! immutability is really useful for collections outside of supporting issolates, etc

Copy link

Choose a reason for hiding this comment

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

True, but if we get the semantics wrong we may find ourselves trying to retrofit immutables onto isolates. So I really prefer that we consider isolates immediately.

Copy link
Member

Choose a reason for hiding this comment

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

Sure. Just saying that it's nice on its own, too.

Copy link
Member

Choose a reason for hiding this comment

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

There is a difference between shallowly unmodifiable and deeply immutable objects, and we probably want both.

Copy link
Member Author

Choose a reason for hiding this comment

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

What does shallow immutability give you that we don't already have with final fields?

## Javascript

Currently, isolates are not supported in Javascript. If we revisit that, we are
unlikely to be able to support this in full on the web. It is possible that we
Copy link
Member

Choose a reason for hiding this comment

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

I assume that all Immutable types will work fine in JS – just the isolate APIs won't be available.

I'm sure there will be places where we can generate better JS if we know things are immutable, though – so I still consider this a "feature" – even for web-only code.

CC @rakudrama @jmesserly for comments...

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it's the communication that wouldn't work out of the box.

Copy link

Choose a reason for hiding this comment

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

I think cloning objects should be a sufficient polyfill. You don't get the performance benefits from memory sharing, but you still get the benefit of a second isolate (i.e. web worker).

@leafpetersen
Copy link
Member Author

would this support if(Value<String>() is immutable) {...} checks like the "Alternative syntax"?

Not without explicitly adding support in some way. This may be another argument for using a marker interface instead of constraints.

@yjbanov
Copy link

yjbanov commented Dec 5, 2018

I really like the semantics this proposal is going for, and I'm hopeful that the GC and isolate communication are solvable issues. Overall, very excited about this proposal!

Comments

I'm worried about the syntactic punishment we'd be subjecting the users of immutable classes and collections. It is an especially strange compromise to make given that the proposal requires a language change. I'd imagine that with a language change we could offer a much better syntax for this.

A few suggestions:

  • Introduce keyword data that replaces class and means "data class".
  • In generics immutability constraint is spelled out as simply <data T>.
  • final is implicit in data classes.
  • prefer "Data" prefix in class names.

("value" is also a good option; despite the extra character it is easier to type than "data", which is typed using one hand on a qwerty keyboard)

BEFORE:

class Value<T> extends Scalar<T> implements Constant is immutable
  where T is immutable {
}

foo<S, T, where T is immutable>(Value<T> v) {
}

ImmutableList;
ImmutableSet;
ImmutableMap;

AFTER:

data Value<data T> extends Scalar<T> {
}

foo<S, data T>(Value<T> v) {
}

DataList;
DataMap;
DataSet;

For better heap semantics consider also:

  • operator== and hashCode are transparently provided by the language.
  • fields are non-nullable by default.

Why operator== and hash code? A fundamental problem arises when expressing state changes by passing immutable objects between isolates: given an immutable object representing the state of an entity and a second immutable object representing previous state of the same entity, compute the update. Typically you do not have a single object, but a tree of objects. Comparing two trees has a much better big-O metric when equal sub-trees are also the same objects. Typically a small update in a large tree can be computed in O(log(N)) where N is the size of the tree. If objects in the shared heap are canonicalized we get the desired characteristic because of equality can be check by simple pointer comparison. IOW, there's no difference between operator== and identical.

Why non-nullability? This is to reduce GC and canonicalization pressure. For example, non-null bool, int, double (and hopefully even String) can be inlined into the object representation. This makes computation of hash code and equality check during canonicalization much cheaper. This also reduces object count for GC to traverse. Finally, assignment from non-data object into data object are simplified as we only need to write the value of the object into the field rather than clone the object from non-shared region into the shared region.

@matanlurey
Copy link
Contributor

I tend to agree with @yjbanov (I guess that isn't news). Some other thoughts:

  • I am not sure what a marker interface (is Immutable) buys users:
void example(List<Player> players) {
  if (players is Immutable) {
    // ??? Why
  }
}
  • I wish that we could have immutable collections that can't be upcasted to mutable ones:
void insertTopPlayer(List<Player> players) {}

void main() {
  ImmutableList<Player> p = [Player('Leaf')];
  insertTopPlayer(p); // Statically OK, runtime OK, until "<List>.add" is called.
}

@leafpetersen
Copy link
Member Author

@yjbanov

  • Introduce keyword data that replaces class and means "data class".

I was thinking along these lines last night, I think it could be a good idea. I do think you want to be able to subclass/implement regular classes from data classes, subject to appropriate constraints, but I think that fits with this.

If objects in the shared heap are canonicalized we get the desired characteristic

Do you mean by the programmer, or by the system? This spec doesn't provide for the system to do that - we'd need to explicitly call out different identity semantics for immutable classes.

@leafpetersen
Copy link
Member Author

  • I am not sure what a marker interface (is Immutable) buys users:

As I've specified it, you can only SendPort.share immutable objects. If you want to be able to put immutable objects in collections with other objects, and then later share them, you would need to have some way of recovering the immutability. If you had some known immutable super-interface you could cast to that, but otherwise you'd be stuck.

  • I wish that we could have immutable collections that can't be upcasted to mutable ones:

Me too.. but my guess is that the ability to reuse all of the existing read only APIs that expect List et al outweighs this. I'm open to discussing it thought.

@matanlurey
Copy link
Contributor

matanlurey commented Dec 5, 2018

@leafpetersen: Thanks!

I do think you want to be able to subclass/implement regular classes from data classes

Doesn't that run into the same problem, more or less, as my up-casting issue?

import 'animal.dart' show Animal;
data ImmutableAnimal implements Animal { ... }

I can see some value in this, for compatibility. I wonder if it is worth it.

Are there concrete cases you think implementing non-immutable/value objects buys users a lot?

As I've specified it, you can only SendPort.share immutable objects.

Since this is, currently, a VM only detail, is it important to make that user-visible? That is, can the VM check (through whatever mechanism it deems best) that objects are transferable, similar to how the V8/JS VM works, and not need a specific type signature?

This is a pretty advanced feature (all things considered), so I imagine:

abstract class SendPort {
  void share(Object transferable);
}

... is fine. You lose (some) static checks, sure.

I imagine if desired, dartanalyzer could special-case this API or you could go with the where constraint.

reuse all of the existing read only APIs that expect List et al outweighs this

As you can probably imagine, @srawlins @yjbanov and others chatted about this at lunch 😄.

The closest comparison I have right now is @davidmorgan's Built{List|Map|Set}, which chose not to implement {List|Map|Set}, and I don't see users complaining or needing this feature. I actually added to .as{List|Map|Set} to help for compatibility. I imagine we could do the same. Consider:

abstract class ImmutableList<T> implements Iterable<T> {
  /// Returns a read-only [List] view into the underlying collection.
  ///
  /// Unlike [ImmutableList], this interface is compatible with APIs that expect [List]. Any operation that
  /// attempts to write (for example, [List.add]) will throw an [UnsupportedError]. See [UnmodifiableList].
  UnmodifiableList<T> asList();
}

I really would like to avoid seeing a lot of user code like this:

void useList(List<String> names) {
  if (names is ImmutableList<String>) {
    names = List.of(names);
  }
  // Add items to names and use it.
}

@matanlurey
Copy link
Contributor

Also, if operator== and hashCode is implemented, it's possible we (AngularDart) could use this for change detection. I know that isn't necessarily enticing enough for this language feature, but I think it's definitely a cool way to reduce the amount of code generation necessary:

data MatButtonProps {
  String title;
  bool isRaised;
}

void checkProps(MatButtonProps a, MatButtonProps b) {
  if (a != b) {
    updateProps(b);
  }
}

@leafpetersen
Copy link
Member Author

Meta-comment: there's a lot of great feedback here - but I'd suggest that anything that isn't specifically actionable on this text (as in stuff I should fix before landing this draft) would be better put here since PR comments tend to disappear into the ether as soon the PR lands.

@leafpetersen
Copy link
Member Author

Doesn't that run into the same problem, more or less, as my up-casting issue?

Yes. My sense is that, if you don't want to implement a mutable interface with your immutable class, then... don't. :) But the flexibility might be worth it. On the other hand, @munificent points out that this makes it breaking to add a private mutable field to a class (since you might be subclassed by an immutable class). So there is a tradeoff there - I'm open to being convinced.

The closest comparison I have right now is @davidmorgan's Built{List|Map|Set}, which chose not to implement {List|Map|Set},

That's good input, thanks.

One question that feeds into this is whether the ability to imperatively initialize immutable lists is important. I specified it that way based on experiences in other languages with immutable vectors, where you sometimes have to jump through hoops to efficiently create an immutable array.

If we want that ability, then there needs to be some mutation API, and there's some convenience in re-using the List API.

@matanlurey
Copy link
Contributor

matanlurey commented Dec 6, 2018

So there is a tradeoff there - I'm open to being convinced.

I'll try :). One nice thing here is we don't need language features do to some usability features on how an ImmutableList would "feel" if it had a few different properties (we could create a package:, and experiment a bit):

  1. If ImmutableList implements List
  2. If ImmutableList had an implicit conversion to List
  3. If ImmutableList had an explicit conversion asList()

That's good input, thanks.

No thank you for starting this discussion!

... whether the ability to imperatively initialize immutable lists is important.

I think it's cool. I don't know if its important. It's also not clear to me if this could be a future enhancement (i.e. get declarative and-or explicit conversion from List in 2.N, and add imperative initialization in 2.N+F).

At least for the Flutter case, changing List ➡️ ImmutableList would be a breaking change if ImmutableList extends List, so I imagine the Flutter team would want to avoid this for the time being - meaning at least some of the benefit is unlikely to be realized.

Having List extends ImmutableList makes it backwards compatible, at least.

If we want that ability, then there needs to be some mutation API

I guess another option is some sort of generated builder and/or copyWith, as well, FWIW.

@jonahwilliams
Copy link

jonahwilliams commented Dec 6, 2018

Re: ImmutableList implements List

I don't think we should keep backwards compatibility by making static errors into runtime errors. If I'm a user and I'm not sure whether I can use an ImmutableList with an API, I have to either look at its implementation or copy. The current UnmodifiableListView and friends already accomplish this, and they are seldom used.

An alternative: make the UnsupportedError a NoSuchMethodError

  • Add three new interfaces added to the collection hierarchy which implement the non-mutable parts of the current collection, ReadonlyList/Map/Set. The existing collection classes can then extend this (non-breaking).
  • Flutter's children argument can have it's type widened to accept these (non-breaking).
  • An immutable collection can be provided which implements the readonly collection API.

If using immutable collections in Flutter ends up being valuable by catching bugs or leading users to better patterns, we could experiment with a Flutter-specific lint. In the non-flutter case, types can be gradually widened as needed and where possible. I think the benefit of being able to immediately pass an ImmutableList to interfaces accepting Lists would lose out in the long-term to more predictable collection behavior.

@davidmorgan
Copy link
Contributor

I'll add some comments over on the issue.

@leafpetersen
Copy link
Member Author

  • Add three new interfaces added to the collection hierarchy which implement the non-mutable parts of the current collection, ReadonlyList/Map/Set. The existing collection classes can then extend this (non-breaking).

This is definitely possible, but it has the following issues:

  • The primary goal of this proposal is to address isolate communication by making deep immutability a property of the type. Allowing mutable subclasses of immutable types breaks this: instead deep immutability becomes a property of an instance. It's possible that this is a reasonable tradeoff. You would lose static checking for sharing, but that's corner enough not to be too much of an issue. It also means that clients can't rely on something of immutable type actually being immutable, which is a little unfortunate.
  • It also doesn't really directly solve the issue that flutter has with people mutating lists after putting them in the widget tree. You can still do this. A flutter specific lint might help with this though.

@leafpetersen
Copy link
Member Author

I added some notes based on feedback so far. I'm going to land this doc, and will iterate further based on comments. Let's take discussion here.

@leafpetersen leafpetersen merged commit da1c9e6 into master Dec 6, 2018
@kevmoo kevmoo deleted the immutability branch December 6, 2018 21:07
Copy link
Member

@lrhn lrhn left a comment

Choose a reason for hiding this comment

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

I'd consider a more Dartino-like approach where we don't introduce new nominal types, but allow all classes to create immutable objects.
Say: Any object with only final fields and where all fields hold immutable objects, is itself immutable. Maybe only if created using a const constructor.

We can then add immutability to the type system as a type modified: const T is a sub-type of T and of T<:S then const T <: const S. It gives a parallel type hierarchy (like non-nullability). Maybe "const" is not the right word, because it allows non-const values, as long as they are immutable (const is just a way to create and canonicalize some immutable objects at compile-time).

Maybe we can even define classes that are only immutable: const class FinalWord { final String word; const FinalWord(this.word); }. That type only exists as immutable, there is no "mutable" superclass of it.


The SendPort class is extended with a new method `void share<T, where T is
immutable>(T message)` which given a reference to an immutable object graph,
shares that reference with all receivers of the SentPort. Note that the object
Copy link
Member

Choose a reason for hiding this comment

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

What does "shares" mean?
Is it sent on the port, or is it just prepared for sending.

What class does the object have on the "other side"?
Can it access static state? Other types? Can an immutable object have a method like:

foo() => new _Foo(_staticCounter++);

If the receiving isolate has not imported the library declaring the class of the object, can you send it? If you do, what is its run-time type? Which static state can it access?

If the receiving isolate has imported the same library, what is the run-time type of the received object? We have two different instances of the library (they have different static state), so which static state will the object access?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is sent on the port.

I would expect that it can access static state, and every isolate has its own instance of the static state.

Broadly, to your questions, I would expect isolates to share the same code, hence have the same classes loaded, etc. This is what the (very minimal, vague) docs for Isolate.spawn seem to describe? If there are other ways to create isolates that don't share this property, I would suggest that they not be able to access the shared heap.

Copy link
Member

Choose a reason for hiding this comment

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

You can use Isolate.spawnUri to run arbitrary code that doesn't need to have anything in common with the spawning isolate. You can usually only send JSON-like structures between such isolates.

Copy link
Member Author

Choose a reason for hiding this comment

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

in a modifiable state. Mutation operations may be performed on such an instance
up until the first point at which the instance escapes (that is, is captured by
a closure, is assigned to another variable or setter, or is passed as a
parameter). It is a static error if a mutation operation is performed on an
Copy link
Member

Choose a reason for hiding this comment

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

So we have "mutate until frozen" collections.
Do we allow that for other, user written, immutable types too?

(We can perhaps get away with this by saying that you can only mutate the object using cascades on the object creation expression, and that mutating methods on an immutable object may not leak this in any way. Still sounds complicated).

Copy link
Member Author

Choose a reason for hiding this comment

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

I raised this question below - if there's need for it we can do it. It's basically a limited use of typestate. How limited depends on how much power we need or don't need.

always initialized in an umodifiable state.

Question: Is this functionality needed? With spread collections, many patterns
will be expressible directly as a literal.
Copy link
Member

Choose a reason for hiding this comment

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

Likely not needed, and it's a very fragile functionality, so I'd prefer to avoid it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe. @munificent agrees with you. After years of struggling with vector initialization in functional languages.... I'm a little more skeptical.

### Alternative collection approach

Instead of making `ImmutableList` a subtype of `List`, we could make it either
an unrelated type, or a supertype of `List`.
Copy link
Member

Choose a reason for hiding this comment

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

As you say below, it can't be a supertype if subtypes of immutable types must be immutable. If that isn't true, then the type system won't be the thing enforcing immutability.
(It's also a good reason why you should never name a type negatively, because a restriction is usually something you can remove in a sub-class, and new MutableList() is ImmutableList being true is just weird).

We can add a supertype of List called Sequence, and then let ImmutableList implement Sequence. We probably won't because it introduces a new super-type of List that code authors should then change all their read-only List accepting functions to using.

So ImmutableList shoud either be a subtype of List or be unrelated to List.
The former allows it to be used with code that currently accepts a list.
The latter allows it to have a different API, say with functional updates. I don't think introducing a completely new API is worth it.

the case of collections). Alternatively, we could simply not enforce deep
immutability statically, and instead dynamically traverse an object grap before
sharing it to check for immutability. This is expensive, but perhaps marginally
less so than copying.
Copy link
Member

Choose a reason for hiding this comment

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

The Dartino approach was to have one bit on each object telling whether it's immutable.
(That could also be based on which GC page it is allocated in, or something similar).

When you create a new object using a const constructor, or an .unmodifiable constructor of a system collection, then the bit gets set if all the fields/elements/entries are immutable.
You never need to do a deep check, but you do one more computation for each component object when creating a composite object.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I thought about writing this up as an alternative proposal, may still do (or feel free). I'm not very enthusiastic about it though. It makes every allocation in the program a little slower (or potentially a lot slower in JS), even if no immutability is used.

with this.

A benefit of this is that changing APIs (especially Flutter APIs) to take
`ImmutableList` as an argument would be non-breaking.
Copy link
Member

Choose a reason for hiding this comment

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

It's not clear how this would work. A type annotation of ImmutableList should accept the subtype _GrowableList.
If it's not type based, then it should probably not show up in the type system at all.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I don't see a coherent story here, but added it based on discussion for future reference.

@leafpetersen
Copy link
Member Author

I'd consider a more Dartino-like approach where we don't introduce new nominal types, but allow all classes to create immutable objects.

I'm open to this, but I'd like to see it worked out. The key point of what I'm specifying here is that immutable objects can be allocated directly into a shared heap. You can instead copy into a shared heap, or do something else, but what that something else is needs to be worked through.

Say: Any object with only final fields and where all fields hold immutable objects, is itself immutable. Maybe only if created using a const constructor.

const is certainly not enough.

We can then add immutability to the type system as a type modified: const T is a sub-type of T and of T<:S then const T <: const S. It gives a parallel type hierarchy (like non-nullability).

Maybe? I don't know what this means though. What are the inhabitants of const T? Deeply immutable T instances? How does this get enforced? It feels like if you want static enforcement, then it needs to implicitly push structurally down the class hierarchy. If it only means shallow immutability... what is it buying you over just having a class with final fields?

I would like to see other proposals worked up (I may try to write up the dynamic version) - this is still very much exploratory.

@yjbanov
Copy link

yjbanov commented Dec 7, 2018

@lrhn, the Dartino approach has some theoretical appeal in that you are kind of like composing immutability with existing concepts. However, I think there are downsides. This proposal is about sharing memory and memory implies data and not behavior. Existing class semantics are great at expressing behaviors, such as services, State objects, particularly mutable objects. By default classes are mutable, not comparable, have poor memory layout controls, which biases the entire class system towards non-data use. Objects representing data favor different, almost opposite properties. You almost always want immutability. You frequently compare one piece of data to another. You always want data to take as little memory space as possible.

So in practice you will rarely find it useful to take any random class and use it in two modes, a mutable non-shared mode and immutable shared mode. Instead you are more likely to design the class for one use-case or the other.

I list some of the desired properties of immutable objects in #125 (comment), and I do not see a clear path there if we start with existing classes.

However, I'd still be curious to see a complete proposal.

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.

None yet

9 participants