search

dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.11k stars 1.56k forks source link

Consider making breaking change of making `dart:io` APIs return deeply immutable bytes. #50069

Open mkustermann opened 2 years ago

mkustermann commented 2 years ago

The VM has support for sharing deeply immutable typed data across isolates. Though it is cumbersome to create such deeply immutable typed data - especially if one wants to avoid an additional copy (see e.g. dart-lang/sdk/issues/50068).

Many typed data instances originate from outside dart code (e.g. loaded from filesystem, socket / http, etc). When that data is read, it is very rarely modified.

So I propose making all dart:io APIs return deeply immutable bytes. It would be a breaking change due to a very small percent of use cases where such data is actually modified, those places would need to be updated.

Though the benefit (all dart:io returned bytes are sharable across isolates by-pointer) seems to outweigh the cost to me (very rare case of modifying such bytes).

The breaking change would not necessarily imply we have to change the actual types of the API (though that would be an option)

/cc @lrhn opinions?

lrhn commented 2 years ago

If you can pull it off, I have no problem with the change. It's a breaking change. Whether someone is actually broken depends on whether you're right that people don't actually change the data being read in.

Do we have a proper unmodifiable ByteBuffer to back this?

mraleph commented 2 years ago

We could consider if there is a path to making this opt-in or configurable in some way, e.g. through a separate set of APIs or through some annotation or language version (somehow)? Then we don't have to bother about it being breaking.

mkustermann commented 2 years ago

Do we have a proper unmodifiable ByteBuffer to back this?

We do - at least in the VM.

The trick is to generate a backing, a _UnmodifiableByteBufferView on top of it and ensure nobody can access the backing store (thereby ensuring nobody can write to it).

Our C API exposes this functionality, see NewExternalTypedData(..., unmodifiable)

We would a similar mechanism for UnmodifiableUint8List.fromChunks() (see https://github.com/dart-lang/sdk/issues/50068)

We could consider if there is a path to making this opt-in or configurable in some way, e.g. through a separate set of APIs or through some annotation or language version (somehow)? Then we don't have to bother about it being breaking.

I think doing so may be feasible, at the expense of clean API. Though for us to actually benefit, we'd need to go through dart:io and all code that reads from files and change that to opt-in to getting immutable bytes. That seems like a lot of changes.

If we on the other hand, decided to change the semantics to return immutable bytes, we only need to update very few places that actually need a writable bytes. The price we pay here is that for those use cases, we have to make mutable copies.

@lrhn wdyt?

Someone could do a trial of this in g3 and see how many places would need updating (one place I'm aware of is the WebSocketTransformer code)