Improve enum decoding -- alternative

[osa1]

When decoding an enum value, we call a callback in the enum field's FieldInfo. The callback then indexes a map mapping enum numbers to Dart values.

When these conditions hold:

• The known enum numbers are all positive. (so that we can use a list and index it with the number)

• The known enum numbers are more than 70% of the large known enum number. (so that the list won't have a lot of null entries, wasting heap space)

We now generate a list instead of a map to map enum numbers to Dart values.

Similar to the map, the list is runtime allocated. No new code generated per message or enum type.

AOT benchmarks:

• Before: protobuf_PackedEnumDecoding(RunTimeRaw): 47585.14 us.
• After: protobuf_PackedEnumDecoding(RunTimeRaw): 38974.566666666666 us.
• Diff: -18%

Wasm benchmarks:

• Before: protobuf_PackedEnumDecoding(RunTimeRaw): 52225.0 us.
• After: protobuf_PackedEnumDecoding(RunTimeRaw): 34283.33333333333 us.
• Diff: -34%

Alternatives considered:

• Improve enum decoding #980 uses a map always, but eliminates the valueOf closure.
• Improve enum decoding -- alternative with binary search instead of map lookup #985 uses a list always, and does binary search in the list when the list is "shallow".
• Improved enum decoding 4 #987 is the same as Improve enum decoding -- alternative with binary search instead of map lookup #985, but instead of calling the valueOf closure it stores an extra field in FieldInfos for whether to binary search or directly index.

These are all slower than the current PR.

---

cl/757724889

[osa1]

@mkustermann this change performs better, and while it's also a breaking change (adds a new member to a public class that can be extended), in practice this won't break anything and it doesn't require major version bump.

[mkustermann]

I'd be ok with landing this, but see my comments on the other PR. If they make sense, maybe that could be an even better solution?

[osa1]

@mkustermann I tried:

• Using a map always, but eliminating the closure: Improve enum decoding #980.
• Keeping the closure, but using a list instead of map when the enum is "dense": this PR.
• Keeping the closure, but using a list always, doing binary search or indexing based on whether the enum is "dense": Improve enum decoding -- alternative with binary search instead of map lookup #985.
• Eliminating the closure, doing binary search or indexing based on whether the enum is "dense": Improved enum decoding 4 #987.

Among these, this PR makes most sense to merge, as it's backwards compatible, and it performs the best.

If you could take another look I'd like to merge this.

In the meantime I'll test it internally.

[osa1]

Internal testing reveals a failure which may be related. I'll investigate further before merging this.

[osa1]

Internal testing reveals a failure which may be related. I'll investigate further before merging this.

This turned out to be some unsound user code compiled with dart2js unsound mode. Not an issue with this change. Merging.

[devoncarew]

@osa1 - I'm seeing some issue w/ this change locally when generating the firestore API.

For ProtobufEnum subclasses, the generated code is:

  ...
  static final $core.List<TargetChange_TargetChangeType?> _byValue = $pb.ProtobufEnum.$_initByValueList(values, 4);
  ...

But the ProtobufEnum.$_initByValueList reference doesn't exist:

The method '$_initByValueList' isn't defined for the type 'ProtobufEnum'.

It looks like ProtobufEnum.$_initByValueList() was added to package:protobuf. I think we'll want to rev the package version (to a new minor version) + changelog, and plan to publish concurrent w/ the next protoc_plugin publish.

[osa1]

@devoncarew right, sorry, I should've documented this in the changelog probably.

This changes both the library and plugin. Library changes are backwards compatible, if you don't change your generated classes you can update the library.

But if you update the plugin and re-generate your protos, then you need the new version of the library.

So I think this should be a minor version bump in the library and major one in the plugin.