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

Fix slow handling of protobuf comments #955

Merged
merged 5 commits into from
Feb 6, 2025
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
4 changes: 2 additions & 2 deletions benchmarks/lib/readfile.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

export 'readfile_vm.dart'
if (dart.library.js_interop) 'readfile_js.dart' show readfile;
export 'readfile_vm.dart' if (dart.library.js_interop) 'readfile_js.dart'
show readfile;
4 changes: 4 additions & 0 deletions protoc_plugin/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
and values are now handled to generate Dart `@deprecated` annotations.
([#900], [#908])
* `protoc_plugin` and generated files now require Dart 3.3.0. (#953)
* Fix performance issues when handling documentation comments in protobufs.
([#935], [#955])

[#738]: https://github.com/google/protobuf.dart/issues/738
[#903]: https://github.com/google/protobuf.dart/pull/903
Expand All @@ -22,6 +24,8 @@
[#909]: https://github.com/google/protobuf.dart/pull/909
[#908]: https://github.com/google/protobuf.dart/pull/908
[#953]: https://github.com/google/protobuf.dart/pull/953
[#935]: https://github.com/google/protobuf.dart/pull/935
[#955]: https://github.com/google/protobuf.dart/pull/955

## 21.1.2

Expand Down
39 changes: 29 additions & 10 deletions protoc_plugin/lib/src/shared.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'dart:convert';
import 'dart:io';

import '../protoc.dart';
import '../src/generated/descriptor.pb.dart';

const protobufImportPrefix = r'$pb';
const asyncImportPrefix = r'$async';
Expand All @@ -14,6 +15,20 @@ const grpcImportPrefix = r'$grpc';
const mixinImportPrefix = r'$mixin';

extension FileDescriptorProtoExt on FileGenerator {
bool _listEquals(List<int> a, List<int> b) {
if (a.length != b.length) {
return false;
}
// Note: paths are much likely to share common prefixes than to share common
// suffixes, so it's probably faster to run this loop backwards ;)
for (var i = a.length - 1; i >= 0; i--) {
if (a[i] != b[i]) {
return false;
}
}
return true;
}

/// Convert leading comments of a definition at [path] to Dart doc comment
/// syntax.
///
Expand All @@ -23,18 +38,22 @@ extension FileDescriptorProtoExt on FileGenerator {
/// The output can contain multiple lines. None of the lines will have
/// trailing whitespace.
String? commentBlock(List<int> path) {
final bits = descriptor.sourceCodeInfo.location
.where((element) => element.path.toString() == path.toString())
.toList();

if (bits.length == 1) {
final match = bits.single;
return toDartComment(match.leadingComments);
SourceCodeInfo_Location? singleMatch;
for (final location in descriptor.sourceCodeInfo.location) {
if (_listEquals(location.path, path)) {
if (singleMatch == null) {
singleMatch = location;
} else {
// TODO: evaluate if we should just concatenate all of the matching
// entries.
stderr.writeln('Too many source code locations. Skipping.');
return null;
}
}
}

if (bits.length > 1) {
// TODO: evaluate if we should just concatenate all of the entries.
stderr.writeln('Too many source code locations. Skipping.');
if (singleMatch != null) {
return toDartComment(singleMatch.leadingComments);
}

return null;
Expand Down