Skip to content

Commit

Permalink
Generate separate .pbjson.dart files for constants
Browse files Browse the repository at this point in the history 
This is to reduce the amount of code parsed in Dartium
for the common case where a .proto doesn't define any services.
(Tree shaking does not help us in this case.)

Service definitions still need to import the JSON constants.
But it might also help a bit in this case, since the
dependencies between files are more fine-grained than before.

BUG=
R=sgjesse@google.com

Review URL: https://chromiumcodereview.appspot.com//1941153002 .
  • Loading branch information
Brian Slesinsky committed May 3, 2016
1 parent da26419 commit 63292ef
Show file tree
Hide file tree
Showing 10 changed files with 333 additions and 105 deletions.
2 changes: 1 addition & 1 deletion Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ TEST_PROTO_LIST = \
toplevel_import \
toplevel
TEST_PROTO_DIR=$(OUTPUT_DIR)/protos
TEST_PROTO_LIBS=$(foreach proto, $(TEST_PROTO_LIST), $(TEST_PROTO_DIR)/$(proto).pb.dart)
TEST_PROTO_LIBS=$(foreach proto, $(TEST_PROTO_LIST), $(TEST_PROTO_DIR)/$(proto).pb.dart $(TEST_PROTO_DIR)/$(proto).pbjson.dart)
TEST_PROTO_SRC_DIR=test/protos
TEST_PROTO_SRCS=$(foreach proto, $(TEST_PROTO_LIST), $(TEST_PROTO_SRC_DIR)/$(proto).proto)

Expand Down
58 changes: 28 additions & 30 deletions lib/code_generator.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,9 @@ class CodeGenerator extends ProtobufContainer {
/// for details), and [outputConfiguration] can be used to override where
/// generated files are created and how imports between generated files are
/// constructed (see [OutputConfiguration] for details).
void generate({
Map<String, SingleOptionParser> optionParsers,
void generate(
{Map<String, SingleOptionParser> optionParsers,
OutputConfiguration config}) {

if (config == null) {
config = new DefaultOutputConfiguration();
}
Expand All @@ -42,37 +41,36 @@ class CodeGenerator extends ProtobufContainer {
.fold(new BytesBuilder(), (builder, data) => builder..add(data))
.then((builder) => builder.takeBytes())
.then((List<int> bytes) {
var request =
new CodeGeneratorRequest.fromBuffer(bytes, extensions);
var response = new CodeGeneratorResponse();
var request = new CodeGeneratorRequest.fromBuffer(bytes, extensions);
var response = new CodeGeneratorResponse();

// Parse the options in the request. Return the errors is any.
var options = parseGenerationOptions(
request, response, optionParsers);
if (options == null) {
_streamOut.add(response.writeToBuffer());
return;
}
// Parse the options in the request. Return the errors is any.
var options = parseGenerationOptions(request, response, optionParsers);
if (options == null) {
_streamOut.add(response.writeToBuffer());
return;
}

// Create a syntax tree for each .proto file given to us.
// (We may import it even if we don't generate the .pb.dart file.)
List<FileGenerator> generators = <FileGenerator>[];
for (FileDescriptorProto file in request.protoFile) {
generators.add(new FileGenerator(file));
}
// Create a syntax tree for each .proto file given to us.
// (We may import it even if we don't generate the .pb.dart file.)
List<FileGenerator> generators = <FileGenerator>[];
for (FileDescriptorProto file in request.protoFile) {
generators.add(new FileGenerator(file));
}

// Collect field types and importable files.
link(options, generators);
// Collect field types and importable files.
link(options, generators);

// Generate the .pb.dart file if requested.
for (var gen in generators) {
var name = gen._fileDescriptor.name;
if (request.fileToGenerate.contains(name)) {
response.file.add(gen.generateResponse(config));
}
}
_streamOut.add(response.writeToBuffer());
});
// Generate the .pb.dart file if requested.
for (var gen in generators) {
var name = gen._fileDescriptor.name;
if (request.fileToGenerate.contains(name)) {
response.file.add(gen.generateResponse(config));
response.file.add(gen.generateJsonDartResponse(config));
}
}
_streamOut.add(response.writeToBuffer());
});
}

String get package => '';
Expand Down
14 changes: 13 additions & 1 deletion lib/extension_generator.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@ class ExtensionGenerator {
return _field.needsFixnumImport;
}

/// Adds any imports needed by the Dart code defining this extension.
/// Adds dependencies of [generate] to [imports].
///
/// For each .pb.dart file that the generated code needs to import,
/// add its generator.
void addImportsTo(Set<FileGenerator> imports) {
if (_field == null) throw new StateError("resolve not called");
var typeGen = _field.baseType.generator;
Expand All @@ -51,6 +54,15 @@ class ExtensionGenerator {
}
}

/// Adds dependencies of [generateConstants] to [imports].
///
/// For each .pb.dart file that the generated code needs to import,
/// add its generator.
void addConstantImportsTo(Set<FileGenerator> imports) {
if (_field == null) throw new StateError("resolve not called");
// No dependencies - nothing to do.
}

void generate(IndentingWriter out) {
if (_field == null) throw new StateError("resolve not called");

Expand Down
129 changes: 98 additions & 31 deletions lib/file_generator.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ class FileGenerator extends ProtobufContainer {

final FileDescriptorProto _fileDescriptor;

// The relative path used to import the .proto file, as a URI.
final Uri protoFileUri;

final List<EnumGenerator> enumGenerators = <EnumGenerator>[];
final List<MessageGenerator> messageGenerators = <MessageGenerator>[];
final List<ExtensionGenerator> extensionGenerators = <ExtensionGenerator>[];
Expand All @@ -31,7 +34,14 @@ class FileGenerator extends ProtobufContainer {
/// True if cross-references have been resolved.
bool _linked = false;

FileGenerator(this._fileDescriptor) {
FileGenerator(FileDescriptorProto descriptor)
: _fileDescriptor = descriptor,
protoFileUri = new Uri.file(descriptor.name) {
if (protoFileUri.isAbsolute) {
// protoc should never generate an import with an absolute path.
throw "FAILURE: Import with absolute path is not supported";
}

var defaultMixin = _getDefaultMixin(_fileDescriptor);

// Load and register all enum and message types.
Expand Down Expand Up @@ -117,13 +127,7 @@ class FileGenerator extends ProtobufContainer {
[OutputConfiguration config = const DefaultOutputConfiguration()]) {
if (!_linked) throw new StateError("not linked");

Uri filePath = new Uri.file(_fileDescriptor.name);
if (filePath.isAbsolute) {
// protoc should never generate a file descriptor with an absolute path.
throw "FAILURE: File with an absolute path is not supported";
}

generateHeader(out, filePath, config);
generateHeader(out, config);

// Generate code.
for (EnumGenerator e in enumGenerators) {
Expand All @@ -137,7 +141,7 @@ class FileGenerator extends ProtobufContainer {
// name derived from the file name.
if (!extensionGenerators.isEmpty) {
// TODO(antonm): do not generate a class.
String className = _generateClassName(filePath);
String className = _generateClassName(protoFileUri);
out.addBlock('class $className {', '}\n', () {
for (ExtensionGenerator x in extensionGenerators) {
x.generate(out);
Expand All @@ -157,24 +161,12 @@ class FileGenerator extends ProtobufContainer {
for (ServiceGenerator s in serviceGenerators) {
s.generate(out);
}

// Put JSON constants at the end of the file.

for (var e in enumGenerators) {
e.generateConstants(out);
}
for (MessageGenerator m in messageGenerators) {
m.generateConstants(out);
}
for (ServiceGenerator s in serviceGenerators) {
s.generateConstants(out);
}
}

/// Prints header and imports.
void generateHeader(IndentingWriter out, Uri filePath,
void generateHeader(IndentingWriter out,
[OutputConfiguration config = const DefaultOutputConfiguration()]) {
String libraryName = _generateLibraryName(filePath);
String libraryName = _generateLibraryName(protoFileUri);
out.println('///\n'
'// Generated code. Do not modify.\n'
'///\n'
Expand All @@ -199,22 +191,24 @@ class FileGenerator extends ProtobufContainer {
out.println("import '${imp}' show ${symbols.join(', ')};");
}

// Import the .pb.dart files we depend on.
for (var imported in _findProtosToImport()) {
String filename = imported._fileDescriptor.name;
Uri importPath = new Uri.file(filename);
if (importPath.isAbsolute) {
// protoc should never generate an import with an absolute path.
throw "FAILURE: Import with absolute path is not supported";
}
// Create a path from the current file to the imported proto.
Uri resolvedImport = config.resolveImport(importPath, filePath);
Uri resolvedImport =
config.resolveImport(imported.protoFileUri, protoFileUri);
out.print("import '$resolvedImport'");
if (package != imported.package && imported.package.isNotEmpty) {
out.print(' as ${imported.packageImportPrefix}');
}
out.println(';');
}
out.println();

// Services also depend on the json imports.
if (serviceGenerators.isNotEmpty) {
Uri resolvedImport = config.resolveJsonImport(protoFileUri, protoFileUri);
out.print("import '$resolvedImport';");
out.println();
}
}

bool get _needsFixnumImport {
Expand Down Expand Up @@ -267,4 +261,77 @@ class FileGenerator extends ProtobufContainer {

return imports;
}

CodeGeneratorResponse_File generateJsonDartResponse(
OutputConfiguration config) {
if (!_linked) throw new StateError("not linked");

IndentingWriter out = new IndentingWriter();

generateJsonDart(out, config);

Uri filePath = new Uri.file(_fileDescriptor.name);
return new CodeGeneratorResponse_File()
..name = config.jsonDartOutputPathFor(filePath).path
..content = out.toString();
}

void generateJsonDart(IndentingWriter out,
[OutputConfiguration config = const DefaultOutputConfiguration()]) {
Uri filePath = new Uri.file(_fileDescriptor.name);
if (filePath.isAbsolute) {
// protoc should never generate a file descriptor with an absolute path.
throw "FAILURE: File with an absolute path is not supported";
}

var baseLibraryName = _generateLibraryName(filePath);
var libraryName = baseLibraryName + "_pbjson";
out.print('''
///
// Generated code. Do not modify.
///
library $libraryName;
''');

// Import the .pbjson.dart files we depend on.
var importList = _findJsonProtosToImport();
for (var imported in importList) {
Uri resolvedImport =
config.resolveJsonImport(imported.protoFileUri, protoFileUri);
out.print("import '$resolvedImport'");
if (package != imported.package && imported.package.isNotEmpty) {
out.print(' as ${imported.packageImportPrefix}');
}
out.println(';');
}
if (importList.isNotEmpty) out.println();

for (var e in enumGenerators) {
e.generateConstants(out);
}
for (MessageGenerator m in messageGenerators) {
m.generateConstants(out);
}
for (ServiceGenerator s in serviceGenerators) {
s.generateConstants(out);
}
}

/// Returns the generator for each .pbjson.dart file the generated
/// .pbjson.dart needs to import.
Set<FileGenerator> _findJsonProtosToImport() {
var imports = new Set<FileGenerator>.identity();
for (var m in messageGenerators) {
m.addConstantImportsTo(imports);
}
for (var x in extensionGenerators) {
x.addConstantImportsTo(imports);
}
for (var x in serviceGenerators) {
x.addConstantImportsTo(imports);
}
imports.remove(this); // Don't need to import self.
return imports;
}
}
19 changes: 18 additions & 1 deletion lib/message_generator.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,10 @@ class MessageGenerator extends ProtobufContainer {
return false;
}

/// Adds generators of the .pb.dart files that this type needs to import.
/// Adds dependencies of [generate] to [imports].
///
/// For each .pb.dart file that the generated code needs to import,
/// add its generator.
void addImportsTo(Set<FileGenerator> imports) {
if (_fieldList == null) throw new StateError("message not resolved");
for (var field in _fieldList) {
Expand All @@ -212,6 +215,20 @@ class MessageGenerator extends ProtobufContainer {
}
}

/// Adds dependencies of [generateConstants] to [imports].
///
/// For each .pbjson.dart file that the generated code needs to import,
/// add its generator.
void addConstantImportsTo(Set<FileGenerator> imports) {
if (_fieldList == null) throw new StateError("message not resolved");
for (var m in _messageGenerators) {
m.addConstantImportsTo(imports);
}
for (var x in _extensionGenerators) {
x.addConstantImportsTo(imports);
}
}

void generate(IndentingWriter out) {
checkResolved();

Expand Down
Loading

0 comments on commit 63292ef

Please sign in to comment.