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.21k stars 1.57k forks source link

Delegate implementation of dart:io's networking bits... #39104

Open dnfield opened 4 years ago

dnfield commented 4 years ago

...or perhaps provide a new library that does so.

Context: Flutter users are concerned about the added size of including a networking stack that they may not want to use (e.g. pre-existing mobile apps may be expected to use some other networking library https://github.com/flutter/flutter/issues/40220) or that may not be taking full advantage of the OS capabilities (e.g. backgrounding download tasks https://github.com/flutter/flutter/issues/32161, or respecting system proxy settings https://github.com/flutter/flutter/issues/20376).

This also causes some headaches with Flutter for web, since the dart:io network implementation is not very compatible with AJAX/XMLHttpRequest.

A key part of this request is to have a way to buid the Dart VM without including the network bits of dart:io, e.g. all of the socket*.cc and BoringSSL, at least in product mode (where we wouldn't need the observatory anymore).

However, it seems like it would be ideal to be able to just fully delegate the nework calls to some other library in this mode (whether in product mode or not) so that existing code using e.g. HttpClient would not break but could effectively delegate to some other networking library specific to a platform.

It may also work just to have some new dart:net library or somesuch with the same API surface as the existing socket and HTTP implementation, but that delegates.

/cc @zanderso @xster @bkonyi @jonahwilliams who all probably have good thoughts on this.

lrhn commented 4 years ago

Migrating a part of dart:io to, say, dart:net and exporting that from dart:io is definitely a possibility.

That won't solve the problem because you still can't use dart:io without having dart:net available.

We could, perhaps, split dart:io into a number of individual packages:

and export all of them from dart:io, but encourage users to migrate to the individual libraries for "greater portability".

Before doing anything rash, we might want to consider for each of these whether there is a better API that we'd want to expose, and then have dart:io delegate to the better API instead of just exporting it. If the better API is one that can also be supported on the web, then that might be an advantage. The dart:io library design is showing its age. It's not consistent with current design guides and I'm sure we can do it better today. If we keep dart:io for backwards compatibility, then existing code should keep working.

xster commented 4 years ago

also cc @Hixie (I'm not super sure who's driving that effort).

Weren't there customers wanting to supply their own cronet implementation to back Dart networking as well? We should align since they likely have the same technical solution.

devoncarew commented 4 years ago

cc @vsm @leafpetersen as there was some discussion about this at a recent planning meeting.

zanderso commented 4 years ago

I'd be supportive of this change. As for better APIs, the Flutter tool mainly uses package:file and package:platform instead of directly using those parts of dart:io due to the ease of injecting different implementations. If those APIs (and similar ones for network, processes, etc.) were shifted to dart: then you'd get similar benefits at the embedder/platform level rather than just at the app level.

As an alternative approach, which I'm not necessarily endorsing, an embedder (like the engine) could set HttpOverrides.global and/or IOOverrides.global during Isolate setup before main() runs to jam in platform-specific implementations, possibly implemented in embedder-specific, private dart: libraries, like dart:_flutter_cronet, etc.. But I'd really prefer a solution whose benefits go beyond the Flutter engine.

dnfield commented 4 years ago

@zanderso another issue with doing it in an embedder specific way is that apps still need to package in BoringSSL and the rest of the Dart network static on the C++ side, which last I checked was ~.3MB. That becomes a sticking point for some of our customers. I'm hoping that whatever solution is created would not only improve the Dart API surface but also improve options around binary size for people who want to bring their own.

zanderso commented 4 years ago

@dnfield Yeah, there are really two parts to this issue:

  1. Using a different implementation of dart:io APIs (or whatever the new APIs are)
  2. Removing the unwanted implementations.

Since the engine build is currently taking the dart:io implementation off-the-shelf, there's no way to accomplish (2) without the Flutter engine doing something embedder specific. The embedder specific thing that the engine does could be supplying its own implementation of a new dart:net library, or it could be supplying an IOOverrides and not using the standalone VM's *_patch.dart files and not linking in the things that they refer to.

jonahwilliams commented 4 years ago

If I'm understanding Option 2 in the context of a slimmed down Flutter, that would mean something like entirely removing HttpClient/HttpServer along with the associated native bits?

That has a lot of unfortunate side effects. In general, it would make it very difficult to prevent accidental breakages if a downstream dependency of the a SDK. Consider the follow case:

  1. Flutter does not use class A from dart:io.
  2. Customized Flutter/dart SDK is forked, removes class A to save code size.
  3. Flutter begins using class A
  4. Customized Flutter/Dart SDK is updated and breaks due to framework usage of A.

The above scenario is only preventable if Flutter essentially freezes the usage of dart core libraries. That would not be conducive to taking advantage of updates to dart core libraries.

dnfield commented 4 years ago

The goal would be to have some other implementation in use, e.g. NSURLConnection on uOS

jonahwilliams commented 4 years ago

That sounds like option 1 though, the interfaces stay but the implementation changes

zanderso commented 4 years ago

If I'm understanding Option 2 in the context of a slimmed down Flutter, that would mean something like entirely removing HttpClient/HttpServer along with the associated native bits?

I was assuming it meant backing e.g. the dart:io HttpClient/HttpServer with a different implementation, possibly in a new dart:net library, where the new implementation in dart:net would have further capabilities like @dnfield mentions in the issue description. So the goal for (1) in https://github.com/dart-lang/sdk/issues/39104#issuecomment-546456812 is to add new implementations, and the goal for (2) is to remove the redundant native bits (like BoringSSL). For both, the Dart interfaces remain, and are unchanged.

mraleph commented 4 years ago

/cc @a-siva

dnfield commented 4 years ago

I'm going to work on a design doc for this.

zoeyfan commented 4 years ago

@mraleph , is it reasonable to track this under https://github.com/dart-lang/sdk/issues?q=is%3Aopen+is%3Aissue+label%3Avm-aot-code-size?

zanderso commented 4 years ago

/cc @mehmetf who is looking at some changes to the HttpClient, and might be interested in this.

mraleph commented 4 years ago

@zoeyfan that label is just about the size of compiled Dart code - so it does not fit this particular issue which is more about the size of the runtime system.

mehmetf commented 4 years ago

There has been numerous discussions around this internally. I have asked help from vsm to help organize this work.

Flutter's embedding of Dart on a given platform should match the core native characteristics of the platform. Doing this via plugins makes little sense to me. It should be directly provided by Flutter. In here, "core" means the categories that Lasse enumerated above (network, process, file).

Recent examples where we had to create inferior solutions:

a-siva commented 4 years ago

Before doing anything rash, we might want to consider for each of these whether there is a better API that we'd want to expose, and then have dart:io delegate to the better API instead of just exporting it. If the better API is one that can also be supported on the web, then that might be an advantage. The dart:io library design is showing its age. It's not consistent with current design guides and I'm sure we can do it better today.

@lrhn could you point us to a document which talks about the current design guidelines and how the dart:io API can be made better.

lrhn commented 3 years ago

I don't believe there is any such document, it's just API design choices that are either internally inconsistent, or that are not following patterns that have become de-facto standard for writing Dart code.

For example, the IO library has several classes which extend Stream, which is almost never done elsewhere ("favor composition over inheritance" is probably the most common general rule violated by dart:io). An HttpRequest is a steam sink that you can write headers into, rather than having a headers sink. Similarly, the IO sinks have every possible write themselves, both bytes and strings, instead of just being byte sinks and providing an easy ways to wrap them into string sinks with a specific encoding.

There is the write and writeSync distinction with file I/O, which is not something I have a good solution for, but it's not great API design. The File class (and other file system entities) seems backwards - you create a File object first, before checking whether the path is a file at all. The File class doesn't represent a file, it really represents a path which may or may not point to a file (or a directory, or nothing). But the Directory class, which also represents a path, just with other intended operations, does not help you with actual path operations. You have to do File(path.join(dir.path, "filename")) instead of just, say, dir.file("filename"). So, the classes are inconsistent in whether they represent an abstract path or an on-disk entity, which makes them do both things half-heartedly. It's more of a grab-bag of functionality related to what you want the path to represent.

It's not bad, just not very consistent with almost all other Dart code.

brianquinlan commented 2 years ago

cupertino_http is a new experimental Flutter plugin that provides access to Apple's Foundation URL Loading System - which supports HTTP/3, honors iOS VPN/Proxy settings, etc.

cupertino_http has the same interface as package:http Client so it is easy to use in a cross-platform way. For example:

late Client client;
if (Platform.isIOS) {
  final config = URLSessionConfiguration.ephemeralSessionConfiguration()
    # Do whatever configuration you want.
    ..allowsCellularAccess = false
    ..allowsConstrainedNetworkAccess = false
    ..allowsExpensiveNetworkAccess = false;
  client = CupertinoClient.fromSessionConfiguration(config);
} else {
  client = IOClient(); // Uses an HTTP client based on dart:io
}

final response = await client.get(Uri.https(
    'www.googleapis.com',
    '/books/v1/volumes',
    {'q': 'HTTP', 'maxResults': '40', 'printType': 'books'}));

I would really appreciate it if you can try cupertino_http out and see if it fixes some of the problems listed above for you for iOS.

Comments or bugs in the cupertino_http issue tracker would be appreciated!