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

Improve packed field decoding #959

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from
Draft

Improve packed field decoding #959

wants to merge 12 commits into from

Conversation

osa1
Copy link
Member

@osa1 osa1 commented Feb 10, 2025

  • Inline _readPacked manually and _withLimit with a pragma to eliminate
    closure allocation and calls in packed decoding loops.

  • Introduce PbList._addUnchecked to add to the list without checking the
    value for validity and list for mutability.

  • When decoding a packed field, check the list mutability once, instead of for
    every element.

  • When decoding a packed scalar field, don't check for value validity.

    For scalar fields we need to make sure the field value is not null, which is
    already guaranteed in the call sites as e.g. input.readDouble doesn't
    return nullable.

  • Sprinkle a bunch of prefer-inlines to make sure VM will inline one liners.

Looking at the optimized flow graph, this should be quite fast compared to the
current version. However I can't benchmark this yet as none of the benchmarks
use packed fields.

I will update the benchmarks first, then revisit this PR.


TODO: I think "check" functions in PbLists can be completely eliminated for the scalars where we only check for nulls:

CheckFunc getCheckFunction(int fieldType) {
switch (fieldType & ~0x7) {
case PbFieldType._BOOL_BIT:
case PbFieldType._BYTES_BIT:
case PbFieldType._STRING_BIT:
case PbFieldType._DOUBLE_BIT:
case PbFieldType._ENUM_BIT:
case PbFieldType._GROUP_BIT:
case PbFieldType._MESSAGE_BIT:
case PbFieldType._INT64_BIT:
case PbFieldType._SINT64_BIT:
case PbFieldType._SFIXED64_BIT:
case PbFieldType._UINT64_BIT:
case PbFieldType._FIXED64_BIT:
// We always use the full range of the same Dart type.
// It's up to the caller to treat the Int64 as signed or unsigned.
// See: https://github.com/google/protobuf.dart/issues/44
return _checkNotNull;
case PbFieldType._FLOAT_BIT:
return _checkFloat;
case PbFieldType._INT32_BIT:
case PbFieldType._SINT32_BIT:
case PbFieldType._SFIXED32_BIT:
return _checkSigned32;
case PbFieldType._UINT32_BIT:
case PbFieldType._FIXED32_BIT:
return _checkUnsigned32;
}
throw ArgumentError('check function not implemented: $fieldType');
}

The generated API for these have non-nullable element type (e.g. PbList<int>) so null checks no longer do anything (since null safety).

@osa1 osa1 marked this pull request as draft February 11, 2025 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant