Skip to content
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

[vm] ☂️ @pragma('vm:deeply-immutable') #55120

Open
3 tasks
dcharkes opened this issue Mar 6, 2024 · 8 comments
Open
3 tasks

[vm] ☂️ @pragma('vm:deeply-immutable') #55120

dcharkes opened this issue Mar 6, 2024 · 8 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. type-enhancement A request for a change that isn't a bug

Comments

@dcharkes
Copy link
Contributor

dcharkes commented Mar 6, 2024

Filing a tracking issue for expanding the feature-set of @pragma('vm:deeply-immutable').

https://dart-review.googlesource.com/c/sdk/+/354902 Introduces basic functionality.

  • Deeply immutable classes can have fields which are deeply immutable.
  • Deeply immutable classes can extend other deeply immutable classes.
  • Allow type parameter types in fields of deeply immutable classes.

Not covered in this CL, but could be covered:

  • Allow using deeply immutable classes across libraries.
  • Allow mixin in deeply immutable classes. (Requires allowing deeply immutable classes across libraries, mixins can't be final or sealed.)
  • Allow record types in fields of deeply immutable classes.
@dcharkes dcharkes added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Mar 6, 2024
@a-siva a-siva added the type-enhancement A request for a change that isn't a bug label Mar 6, 2024
@dcharkes
Copy link
Contributor Author

dcharkes commented Mar 7, 2024

Note that the deeply immutable bit is lost in AppJIT snapshots currently:

This doesn't really matter for the use case with native finalizers (#55062 (comment)), because pointers to native memory cannot be part of snapshots.

copybara-service bot pushed a commit that referenced this issue Mar 7, 2024
This CL introduces a way to mark all instances of a class as deeply
immutable.

In order to statically verify that all instances of a deeply immutable
class are immutable, a deeply immutable classes must have the following
properties:

1. All instance fields must
   1. have a deeply immutable type,
   2. be final, and
   3. be non-late.
2. The class must be `final` or `sealed`. This ensures no
   non-deeply-immutable subtypes are added by external code.
3. All subtypes must be deeply immutable. This ensures 1.1 can be
   trusted.
4. The super type must be deeply immutable (except for Object).

Note that instances of some classes in the VM are deeply immutable
while their class cannot be marked immutable.

* SendPort, Capability, RegExp, and StackTrace are not `final` and
  can be implemented by external code.
* UnmodifiableTypedDataViews do not have a public type. (It was
  recently deprecated.)

See runtime/docs/deeply_immutable.md for more details.

Use case:

This enables attaching a `Dart_FinalizableHandle` to a deeply immutable
object and the deeply immutable object with other isolates in the same
isolate group.

(Note that `NativeFinalizer`s live in an isolate, and not an isolate
group. So this should currently _not_ be used with `NativeFinalizer`s.
See #55062 for making a
`NativeFinalizer.shared(` that would live in an isolate group instead
of in an isolate.)

Implementation details:

Before this CL, the `ImmutableBit` in the object header was only ever
set to true for predefined class ids (and for const objects). After
this CL, the bit can also be set to true for non const instances of
user-defined classes. The object allocation and initialization code has
been changed to deal with this new case. The immutability of a class is
saved in the class state bits. On object allocation and initialization
the immutability bit is read from the class for non-predefined class
ids.

TEST=runtime/tests/vm/dart/isolates/fast_object_copy2_test.dart
TEST=runtime/vm/isolate_reload_test.cc
TEST=tests/lib/isolate/deeply_immutable_*

Bug: #55120
Bug: #54885
Change-Id: Ib97fe589cb4f81673cb928c93e3093838d82132d
Cq-Include-Trybots: luci.dart.try:vm-aot-android-release-arm64c-try,vm-aot-android-release-arm_x64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-debug-x64c-try,vm-aot-mac-release-arm64-try,vm-aot-mac-release-x64-try,vm-aot-obfuscate-linux-release-x64-try,vm-aot-optimization-level-linux-release-x64-try,vm-appjit-linux-debug-x64-try,vm-asan-linux-release-x64-try,vm-checked-mac-release-arm64-try,vm-eager-optimization-linux-release-ia32-try,vm-eager-optimization-linux-release-x64-try,vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64c-try,vm-ffi-qemu-linux-release-arm-try,vm-ffi-qemu-linux-release-riscv64-try,vm-fuchsia-release-x64-try,vm-kernel-linux-debug-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-linux-debug-ia32-try,vm-linux-debug-x64-try,vm-linux-debug-x64c-try,vm-mac-debug-arm64-try,vm-mac-debug-x64-try,vm-msan-linux-release-x64-try,vm-reload-linux-debug-x64-try,vm-reload-rollback-linux-debug-x64-try,vm-ubsan-linux-release-x64-try
Cq-Include-Trybots: dart-internal/g3.dart-internal.try:g3-cbuild-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/354902
Commit-Queue: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
copybara-service bot pushed a commit that referenced this issue Mar 11, 2024
A test exercising isolate group finalizers with `Finalizable` and
`pragma('vm:deeply-immutable')`.

Note that we cannot fully do without C code due to the signature of
`Dart_HandleFinalizer` which also passes a
`void* isolate_callback_data` in addition to the peer.

TEST=tests/ffi/deeply_immutable_c_api_finalizer_test.dart

Bug: #55062
Bug: #55120
Change-Id: Ibaf4899ca678ffb0c5d227ac4f10deb38d49fe6f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/356720
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Hossein Yousefi <yousefi@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
@nielsenko
Copy link

@dcharkes Would it be an idea to have a marker interface that implies deep immutability? That way it would be possible to create generic classes that can be annotated with @pragma('vm:deeply-immutable') by constraining the type arguments on this marker interface. Could be useful for creating higher order immutable data-structures that can be shared efficiently across isolates.

@dcharkes
Copy link
Contributor Author

dcharkes commented Sep 10, 2024

How is that different from allowing a type argument bound which is annotated with @pragma('vm:deeply-immutable') with the regular subtyping rules? We already support having a field and enforce the subtyping rules.

  • Allow type parameter types in fields of deeply immutable classes.

@nielsenko
Copy link

The difference would be that types like int, String, etc. should also implement this marker interface. While I can define my own DeeplyImmutable interface I cannot make the framework classes implement it.

@dcharkes
Copy link
Contributor Author

The difference would be that types like int, String, etc. should also implement this marker interface.

That's a rather invasive change. We've explored something similar in https://github.com/dart-lang/language/blob/main/working/333%20-%20shared%20memory%20multithreading/proposal.md

While I can define my own DeeplyImmutable interface I cannot make the framework classes implement it.

Why not? Because you don't own the classes? or you own them but they are in a different package?

If the latter, we should address:

  • Allow using deeply immutable classes across libraries.

So far we hadn't done that due to the concern of users of libraries breaking libraries by adding new subclasses/implementations that are not deeply immutable. But @mkustermann was recently arguing to remove that restriction.

@nielsenko
Copy link

nielsenko commented Sep 10, 2024

Why not? Because you don't own the classes? or you own them but they are in a different package?

Because I don't own them. The way I read this issue, the cross library feature is already on your list of proposed features.

I read through the multiprocess proposal that was written up by @mraleph, and I see how it is related to his Sharable interface. Anyway, happy to hear you are already thinking about this, even if it may not happen.

@nielsenko
Copy link

For the record this snippet illustrates the issue I'm talking about

@pragma('vm:deeply-immutable')
final class DeeplyImmutable {
  const DeeplyImmutable();
}

@pragma('vm:deeply-immutable')
final class Node<T extends DeeplyImmutable> {
  const Node(this.value, this.next);
  final T value;
  final Node<T>? next;
}

// zero-cost extension types does not allow us to introcuce new marker interfaces
/* 
@pragma('vm:deeply-immutable')
extension type ImmutableInt(int value) implements int, DeeplyImmutable {}
*/
// so we need a full wrapper class like:
@pragma('vm:deeply-immutable')
final class ImmutableInt extends DeeplyImmutable {
  const ImmutableInt(this.value);
  final int value;
}

void main(List<String> arguments) {
  // This will not compile, as int don't implement the marker interface
  // final n = Node(1, Node(2, null));

  // This works, but introduces a wrapper object for every int
  final n = Node(ImmutableInt(1), Node(ImmutableInt(2), null));

  print(n.next?.value.value);
}

@mkustermann
Copy link
Member

The difference would be that types like int, String, etc. should also implement this marker interface. While I can define my own DeeplyImmutable interface I cannot make the framework classes implement it.

For now this is a VM-specific feature and use cases are limited. This is why we decided to go initially with a @pragma(). If we made a marker interface and made String / ... implement it - it would become effectively a language feature - that would require much more discussion with language & library teams, ...

If the shared memory multithreading effort (currently in prototyping stage) will eventually land for end users, then there may not actually be a need for this deep immutability anymore - as what would be annotated with deep immutability can then be instead annotated with sharability (and get the same effect when sending across isolates - namely that they are shared).

So I think it makes sense for us to wait for the shared memory multithreading effort to be further along. If that effort doesn't result in shipping anything, the argument of making deep immutability more of a library & language feature is stronger.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants