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

Opportunity to improve the json decoding performance #2238

Closed
knaeckeKami opened this issue Jun 7, 2024 · 3 comments · Fixed by #2239
Closed

Opportunity to improve the json decoding performance #2238

knaeckeKami opened this issue Jun 7, 2024 · 3 comments · Fixed by #2239
Labels
e: performance Improvements or additions to performance p: dio Targeting `dio` package s: best practise It's the best way to do something so far s: feature This issue indicates a feature request

Comments

@knaeckeKami
Copy link
Contributor

knaeckeKami commented Jun 7, 2024

Request Statement

Hi there!

I experimented a bit with the hidden _JsonUtf8Decoder which is created when fusing the utf8 decoder with the json decoder.

This significantly improves the decoding performance.

I created a benchmark here.

It shows an improvement of around ~10x in AOT mode on sufficiently large json responses. (around 64 KiB).

Here's the improved Transformer:

import 'dart:convert';
import 'dart:typed_data';

import 'package:dio/dio.dart';

class UTF8JsonTransformer extends SyncTransformer {
  /// on VM, this creates a special fast decoder.
  /// see https://github.com/dart-lang/sdk/blob/5b2ea0c7a227d91c691d2ff8cbbeb5f7f86afdb9/sdk/lib/_internal/vm/lib/convert_patch.dart#L54
  final decoder = const Utf8Decoder().fuse(const JsonDecoder());

  UTF8JsonTransformer();

  @override
  Future<dynamic> transformResponse(
    RequestOptions options,
    ResponseBody responseBody,
  ) async {
    final responseType = options.responseType;
    // Do not handled the body for streams.
    if (responseType == ResponseType.stream) {
      return responseBody;
    }

    // Return the finalized bytes if the response type is bytes.
    if (responseType == ResponseType.bytes) {
      final chunks = await responseBody.stream.toList();
      final responseBytes =
          Uint8List.fromList(chunks.expand((c) => c).toList());
      return responseBytes;
    }

    final isJsonContent = Transformer.isJsonMimeType(
      responseBody.headers[Headers.contentTypeHeader]?.first,
    );

    if (isJsonContent) {
      final stream = responseBody.stream;
      final decodedStream = decoder.bind(stream);
      final decoded = await decodedStream.toList();
      assert(decoded.length == 1);
      return decoded.first;
    }

    // If the response is not JSON, we should return the response as a string

    final chunks = await responseBody.stream.toList();
    final responseBytes = Uint8List.fromList(chunks.expand((c) => c).toList());

    return utf8.decode(responseBytes, allowMalformed: true);
  }
}

Note: you need a recent version of Dart to see these performance improvements.

This would be a nice improvement for Dio. Who doesn't like free improved performance?

A caveat is, that this is not compatible with the default BackgroundTransformer, as the callback in this transformer takes a String, not a Uint8List.
To get the benefits there, a change in the API would be required (or we could add a new BackgroundTransformer class).

Would you be open to a PR that integrates these changes in the existing SyncTransformer?

Solution Brainstorm

No response

@knaeckeKami knaeckeKami added the s: feature This issue indicates a feature request label Jun 7, 2024
@AlexV525
Copy link
Member

AlexV525 commented Jun 8, 2024

Is the fuse method compatible with Dart 2.18?

@AlexV525
Copy link
Member

AlexV525 commented Jun 8, 2024

Is the fuse method compatible with Dart 2.18?

Looks like it does. Making a new transformer here would be an acceptable solution and it won't be a breaking change if their behaviors are the same.

@AlexV525 AlexV525 added p: dio Targeting `dio` package s: best practise It's the best way to do something so far e: performance Improvements or additions to performance labels Jun 8, 2024
@knaeckeKami
Copy link
Contributor Author

Is the fuse method compatible with Dart 2.18?

Yes, the fuse method has been there for a very long time, even before Dart 2.0. But by default, it does not do anything drastically different from just calling both converters sequentially.

The new, faster encoder that is obtained specifically by fusing utf8 + json is newer, and recently it got improvements to be even faster.

Looks like it does. Making a new transformer here would be an acceptable solution and it won't be a breaking change if their behaviors are the same.

Great, I'll prepare a PR then!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
e: performance Improvements or additions to performance p: dio Targeting `dio` package s: best practise It's the best way to do something so far s: feature This issue indicates a feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants