Skip to content

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

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

Open
mkustermann opened this issue Sep 28, 2022 · 3 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries.

Comments

@mkustermann
Copy link
Member

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?

@mkustermann mkustermann added the area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. label Sep 28, 2022
@lrhn
Copy link
Member

lrhn commented Sep 28, 2022

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
Copy link
Member

mraleph commented Sep 28, 2022

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
Copy link
Member Author

mkustermann commented Sep 30, 2022

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 #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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries.
Projects
None yet
Development

No branches or pull requests

3 participants