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.

Sign up for GitHub

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

Jump to bottom

[WIP] Avoid intermediate values when serializing and deserializing proto3 JSON #670

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

[WIP] Avoid intermediate values when serializing and deserializing proto3 JSON #670

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
84c6bc8
Avoid intermediate values when serializing proto3 JSON
osa1 Jun 2, 2022
662efa9
Fix formatting
osa1 Jun 2, 2022
81ddd0c
Fix reserved names test
osa1 Jun 2, 2022
3594c5d
Avoid intermediate JSON objects when merging proto3 JSON messages
osa1 Jun 2, 2022
a64b2f3
Update reserved names
osa1 Jun 3, 2022
a549b57
Update benchmarks to use new functions
osa1 Jun 7, 2022
b8ef37d
Implement well-known type conversions
osa1 Jun 7, 2022
fa4d2ea
Generalize JSON generator and parsers to generate and parse both stri…
osa1 Jun 7, 2022
f155f11
Move writer and reader code to separate files
osa1 Jun 7, 2022
24bffd5
Remove accidentally committed file
osa1 Jun 7, 2022
7c92089
Refactoring
osa1 Jun 7, 2022
6fa98c8
Stubs for well-known types
osa1 Jun 7, 2022
38079b0
Refactor function types, fix a jsontool use error
osa1 Jun 7, 2022
a737185
Rename jsonWriter -> jsonSink
osa1 Jun 7, 2022
9b62b5d
Start updating well-known types
osa1 Jun 8, 2022
d44a919
Update rest of the well-known type ser/deser code
osa1 Jun 8, 2022
78d495e
Update plugin for new BuilderInfo arg names
osa1 Jun 8, 2022
c8895c7
Update reserved names
osa1 Jun 8, 2022
e6bee93
Merge remote-tracking branch 'origin/master' into jsontool
osa1 Jun 8, 2022
add7cce
Fix control flow in a few places
osa1 Jun 8, 2022
513d01e
Fix a few bugs
osa1 Jun 9, 2022
951a2c9
Fix a bug
osa1 Jun 9, 2022
00003ce
Update two tests
osa1 Jun 9, 2022
48804f1
Fix a few tests
osa1 Jun 9, 2022
a5a3f98
More bugs
osa1 Jun 9, 2022
dd8cb85
Fix int bounds checks
osa1 Jun 9, 2022
4d43c95
A few fixups
osa1 Jun 9, 2022
6fe0731
Fix bugs, update tests
osa1 Jun 9, 2022
333f16c
Bump jsontool
osa1 Jun 9, 2022
a890105
Remove unused def
osa1 Jun 9, 2022
eaa0d35
Fix error messages
osa1 Jun 9, 2022
14bffa2
Disable a test for now
osa1 Jun 9, 2022
0e28eb3
Fix TODOs
osa1 Jun 9, 2022
02a9960
Handle unexpected input in parseFailure matcher
osa1 Jun 9, 2022
7899d1f
Renaming, docs
osa1 Jun 9, 2022
ee3cf27
More docs
osa1 Jun 9, 2022
617c30c
Bump jsontool
osa1 Jun 16, 2022
712cae7
Merge remote-tracking branch 'origin/master' into jsontool
osa1 Jun 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions protobuf/benchmarks/common.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,35 +101,35 @@ class Factories {
fromBuffer: (List<int> binary) => p2.GoogleMessage1.fromBuffer(binary),
fromJson: (String json) => p2.GoogleMessage1.fromJson(json),
fromProto3JsonString: (String json) =>
p2.GoogleMessage1.create()..mergeFromProto3Json(jsonDecode(json)),
p2.GoogleMessage1.create()..mergeFromProto3JsonString(json),
fromProto3JsonObject: (Object json) =>
p2.GoogleMessage1.create()..mergeFromProto3Json(json)),
'benchmarks.proto3.GoogleMessage1': Factories._(
fromBuffer: (List<int> binary) => p3.GoogleMessage1.fromBuffer(binary),
fromJson: (String json) => p3.GoogleMessage1.fromJson(json),
fromProto3JsonString: (String json) =>
p3.GoogleMessage1.create()..mergeFromProto3Json(jsonDecode(json)),
p3.GoogleMessage1.create()..mergeFromProto3JsonString(json),
fromProto3JsonObject: (Object json) =>
p3.GoogleMessage1.create()..mergeFromProto3Json(json)),
'benchmarks.proto2.GoogleMessage2': Factories._(
fromBuffer: (List<int> binary) => GoogleMessage2.fromBuffer(binary),
fromJson: (String json) => GoogleMessage2.fromJson(json),
fromProto3JsonString: (String json) =>
GoogleMessage2.create()..mergeFromProto3Json(jsonDecode(json)),
GoogleMessage2.create()..mergeFromProto3JsonString(json),
fromProto3JsonObject: (Object json) =>
GoogleMessage2.create()..mergeFromProto3Json(json)),
'benchmarks.google_message3.GoogleMessage3': Factories._(
fromBuffer: (List<int> binary) => GoogleMessage3.fromBuffer(binary),
fromJson: (String json) => GoogleMessage3.fromJson(json),
fromProto3JsonString: (String json) =>
GoogleMessage3.create()..mergeFromProto3Json(jsonDecode(json)),
GoogleMessage3.create()..mergeFromProto3JsonString(json),
fromProto3JsonObject: (Object json) =>
GoogleMessage3.create()..mergeFromProto3Json(json)),
'benchmarks.google_message4.GoogleMessage4': Factories._(
fromBuffer: (List<int> binary) => GoogleMessage4.fromBuffer(binary),
fromJson: (String json) => GoogleMessage4.fromJson(json),
fromProto3JsonString: (String json) =>
GoogleMessage4.create()..mergeFromProto3Json(jsonDecode(json)),
GoogleMessage4.create()..mergeFromProto3JsonString(json),
fromProto3JsonObject: (Object json) =>
GoogleMessage4.create()..mergeFromProto3Json(json)),
};
Expand Down Expand Up @@ -231,7 +231,7 @@ class ToProto3JsonStringBenchmark extends _ProtobufBenchmark {
void run() {
for (final ds in datasets) {
for (final unpacked in ds.unpacked) {
jsonEncode(unpacked.toProto3Json());
unpacked.toProto3JsonString();
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions protobuf/lib/meta.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ const GeneratedMessage_reservedNames = <String>[
'mergeFromJsonMap',
'mergeFromMessage',
'mergeFromProto3Json',
'mergeFromProto3JsonString',
'mergeUnknownFields',
'noSuchMethod',
'runtimeType',
Expand All @@ -52,6 +53,7 @@ const GeneratedMessage_reservedNames = <String>[
'toBuilder',
'toDebugString',
'toProto3Json',
'toProto3JsonString',
'toString',
'unknownFields',
'writeToBuffer',
Expand Down
2 changes: 2 additions & 0 deletions protobuf/lib/protobuf.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import 'dart:math' as math;
import 'dart:typed_data' show TypedData, Uint8List, ByteData, Endian;

import 'package:fixnum/fixnum.dart' show Int64;
import 'package:jsontool/jsontool.dart';
import 'package:meta/meta.dart' show UseResult;

import 'src/protobuf/json_parsing_context.dart';
Expand Down Expand Up @@ -39,6 +40,7 @@ part 'src/protobuf/pb_list.dart';
part 'src/protobuf/pb_map.dart';
part 'src/protobuf/protobuf_enum.dart';
part 'src/protobuf/proto3_json.dart';
part 'src/protobuf/proto3_json_string.dart';
part 'src/protobuf/readonly_message.dart';
part 'src/protobuf/rpc_client.dart';
part 'src/protobuf/unknown_field_set.dart';
Expand Down
19 changes: 19 additions & 0 deletions protobuf/lib/src/protobuf/generated_message.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,13 @@ abstract class GeneratedMessage {
{TypeRegistry typeRegistry = const TypeRegistry.empty()}) =>
_writeToProto3Json(_fieldSet, typeRegistry);

String toProto3JsonString(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider passing an "indent" value

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this really be called writeToProto3Json to maintain consistency?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additions in this PR will be inconsistent unless we want to do breaking changes.

Currently in master branch (and also in the latest release) we have these generators:

  • writeToBuffer
  • writeToCodedBufferWriter
  • writeToJson
  • writeToJsonMap
  • toProto3Json

So the proto3 method is already inconsistent.

Without changing it, we can name the new methods toProto3..., or writeToProto3.... Either way it will be inconsistent.

Should we rename toProto3Json? @sigurdm

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Argh - I see - I introduced that inconsistency - not sure what is the better path forwards.
I guess if we want to be consistent we do:

@Deprecated('Use `writeToProto3JsonMap`');
toProto3Json() => writeToProto3JsonMap();

writeToProto3JsonMap();
writeToProto3Json();

If we prefer to avoid churn over consistency we keep it as toProto3JsonString()....

I guess it's up to your judgement.

{TypeRegistry typeRegistry = const TypeRegistry.empty()}) {
StringBuffer buf = StringBuffer();
_writeToProto3JsonSink(_fieldSet, typeRegistry, buf);
return buf.toString();
}

/// Merges field values from [json], a JSON object using proto3 encoding.
///
/// Well-known types and their special JSON encoding are supported.
Expand Down Expand Up @@ -257,6 +264,18 @@ abstract class GeneratedMessage {
_mergeFromProto3Json(json, _fieldSet, typeRegistry, ignoreUnknownFields,
supportNamesWithUnderscores, permissiveEnums);

void mergeFromProto3JsonString(String jsonString,
{TypeRegistry typeRegistry = const TypeRegistry.empty(),
bool ignoreUnknownFields = false,
bool supportNamesWithUnderscores = true,
bool permissiveEnums = false}) {
final Proto3JsonParserParams params = Proto3JsonParserParams(typeRegistry,
ignoreUnknownFields: ignoreUnknownFields,
supportNamesWithUnderscores: supportNamesWithUnderscores,
permissiveEnums: permissiveEnums);
_mergeFromProto3JsonString(jsonString, _fieldSet, params);
}

/// Merges field values from [data], a JSON object, encoded as described by
/// [GeneratedMessage.writeToJson].
///
Expand Down
3 changes: 2 additions & 1 deletion protobuf/lib/src/protobuf/proto3_json.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ Object? _writeToProto3Json(_FieldSet fs, TypeRegistry typeRegistry) {
dynamic jsonValue;
if (fieldInfo.isMapField) {
jsonValue = (value as PbMap).map((key, entryValue) {
var mapEntryInfo = fieldInfo as MapFieldInfo;
var mapEntryInfo =
fieldInfo as MapFieldInfo; // TODO: move this outside of closure
return MapEntry(convertToMapKey(key, mapEntryInfo.keyFieldType),
valueToProto3Json(entryValue, mapEntryInfo.valueFieldType));
});
Expand Down
Loading