Skip to content

Commit

Permalink
[pigeon] Remove heap allocation in generated C++ code (flutter#6196)
Browse files Browse the repository at this point in the history
Please let me know if I goofed anything, this is my first time contributing to Pigeon!

Addresses flutter/flutter#144042
  • Loading branch information
loic-sharma authored and arc-yong committed Jun 14, 2024
1 parent 401eb68 commit dfa1877
Show file tree
Hide file tree
Showing 9 changed files with 529 additions and 548 deletions.
4 changes: 4 additions & 0 deletions packages/pigeon/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 17.1.1

* Removes heap allocation in generated C++ code.

## 17.1.0

* [kotlin] Adds `includeErrorClass` to `KotlinOptions`.
Expand Down
3 changes: 2 additions & 1 deletion packages/pigeon/example/app/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@ application, rather than in a plugin.

To update the generated code, run:
```sh
dart run pigeon --input pigeons/messages.dart
cd ../..
dart tool/generate.dart
```
30 changes: 14 additions & 16 deletions packages/pigeon/example/app/windows/runner/messages.g.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -141,13 +141,12 @@ const flutter::StandardMessageCodec& ExampleHostApi::GetCodec() {
void ExampleHostApi::SetUp(flutter::BinaryMessenger* binary_messenger,
ExampleHostApi* api) {
{
auto channel = std::make_unique<BasicMessageChannel<>>(
binary_messenger,
"dev.flutter.pigeon.pigeon_example_package.ExampleHostApi."
"getHostLanguage",
&GetCodec());
BasicMessageChannel<> channel(binary_messenger,
"dev.flutter.pigeon.pigeon_example_package."
"ExampleHostApi.getHostLanguage",
&GetCodec());
if (api != nullptr) {
channel->SetMessageHandler(
channel.SetMessageHandler(
[api](const EncodableValue& message,
const flutter::MessageReply<EncodableValue>& reply) {
try {
Expand All @@ -164,16 +163,16 @@ void ExampleHostApi::SetUp(flutter::BinaryMessenger* binary_messenger,
}
});
} else {
channel->SetMessageHandler(nullptr);
channel.SetMessageHandler(nullptr);
}
}
{
auto channel = std::make_unique<BasicMessageChannel<>>(
BasicMessageChannel<> channel(
binary_messenger,
"dev.flutter.pigeon.pigeon_example_package.ExampleHostApi.add",
&GetCodec());
if (api != nullptr) {
channel->SetMessageHandler(
channel.SetMessageHandler(
[api](const EncodableValue& message,
const flutter::MessageReply<EncodableValue>& reply) {
try {
Expand Down Expand Up @@ -203,16 +202,16 @@ void ExampleHostApi::SetUp(flutter::BinaryMessenger* binary_messenger,
}
});
} else {
channel->SetMessageHandler(nullptr);
channel.SetMessageHandler(nullptr);
}
}
{
auto channel = std::make_unique<BasicMessageChannel<>>(
BasicMessageChannel<> channel(
binary_messenger,
"dev.flutter.pigeon.pigeon_example_package.ExampleHostApi.sendMessage",
&GetCodec());
if (api != nullptr) {
channel->SetMessageHandler(
channel.SetMessageHandler(
[api](const EncodableValue& message,
const flutter::MessageReply<EncodableValue>& reply) {
try {
Expand All @@ -239,7 +238,7 @@ void ExampleHostApi::SetUp(flutter::BinaryMessenger* binary_messenger,
}
});
} else {
channel->SetMessageHandler(nullptr);
channel.SetMessageHandler(nullptr);
}
}
}
Expand Down Expand Up @@ -273,12 +272,11 @@ void MessageFlutterApi::FlutterMethod(
const std::string channel_name =
"dev.flutter.pigeon.pigeon_example_package.MessageFlutterApi."
"flutterMethod";
auto channel = std::make_unique<BasicMessageChannel<>>(
binary_messenger_, channel_name, &GetCodec());
BasicMessageChannel<> channel(binary_messenger_, channel_name, &GetCodec());
EncodableValue encoded_api_arguments = EncodableValue(EncodableList{
a_string_arg ? EncodableValue(*a_string_arg) : EncodableValue(),
});
channel->Send(
channel.Send(
encoded_api_arguments, [channel_name, on_success = std::move(on_success),
on_error = std::move(on_error)](
const uint8_t* reply, size_t reply_size) {
Expand Down
13 changes: 5 additions & 8 deletions packages/pigeon/lib/cpp_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -871,11 +871,9 @@ class CppSourceGenerator extends StructuredGenerator<CppOptions> {
scope: api.name,
returnType: _voidType,
parameters: parameters, body: () {
const String channel = 'channel';
indent.writeln(
'const std::string channel_name = "${makeChannelName(api, func, dartPackageName)}";');
indent.writeln(
'auto channel = std::make_unique<BasicMessageChannel<>>(binary_messenger_, '
indent.writeln('BasicMessageChannel<> channel(binary_messenger_, '
'channel_name, &GetCodec());');

// Convert arguments to EncodableValue versions.
Expand All @@ -894,7 +892,7 @@ class CppSourceGenerator extends StructuredGenerator<CppOptions> {
});
}

indent.write('$channel->Send($argumentListVariableName, '
indent.write('channel.Send($argumentListVariableName, '
// ignore: missing_whitespace_between_adjacent_strings
'[channel_name, on_success = std::move(on_success), on_error = std::move(on_error)]'
'(const uint8_t* reply, size_t reply_size) ');
Expand Down Expand Up @@ -972,12 +970,11 @@ class CppSourceGenerator extends StructuredGenerator<CppOptions> {
final String channelName =
makeChannelName(api, method, dartPackageName);
indent.writeScoped('{', '}', () {
indent.writeln(
'auto channel = std::make_unique<BasicMessageChannel<>>(binary_messenger, '
indent.writeln('BasicMessageChannel<> channel(binary_messenger, '
'"$channelName", &GetCodec());');
indent.writeScoped('if (api != nullptr) {', '} else {', () {
indent.write(
'channel->SetMessageHandler([api](const EncodableValue& message, const flutter::MessageReply<EncodableValue>& reply) ');
'channel.SetMessageHandler([api](const EncodableValue& message, const flutter::MessageReply<EncodableValue>& reply) ');
indent.addScoped('{', '});', () {
indent.writeScoped('try {', '}', () {
final List<String> methodArgument = <String>[];
Expand Down Expand Up @@ -1054,7 +1051,7 @@ class CppSourceGenerator extends StructuredGenerator<CppOptions> {
});
});
indent.addScoped(null, '}', () {
indent.writeln('channel->SetMessageHandler(nullptr);');
indent.writeln('channel.SetMessageHandler(nullptr);');
});
});
}
Expand Down
2 changes: 1 addition & 1 deletion packages/pigeon/lib/generator_tools.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import 'ast.dart';
/// The current version of pigeon.
///
/// This must match the version in pubspec.yaml.
const String pigeonVersion = '17.1.0';
const String pigeonVersion = '17.1.1';

/// Read all the content from [stdin] to a String.
String readStdin() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ include(${EPHEMERAL_DIR}/generated_config.cmake)
# https://github.com/flutter/flutter/issues/57146.
set(WRAPPER_ROOT "${EPHEMERAL_DIR}/cpp_client_wrapper")

# Set fallback configurations for older versions of the flutter tool.
if (NOT DEFINED FLUTTER_TARGET_PLATFORM)
set(FLUTTER_TARGET_PLATFORM "windows-x64")
endif()

# === Flutter Library ===
set(FLUTTER_LIBRARY "${EPHEMERAL_DIR}/flutter_windows.dll")

Expand Down Expand Up @@ -92,7 +97,7 @@ add_custom_command(
COMMAND ${CMAKE_COMMAND} -E env
${FLUTTER_TOOL_ENVIRONMENT}
"${FLUTTER_ROOT}/packages/flutter_tools/bin/tool_backend.bat"
windows-x64 $<CONFIG>
${FLUTTER_TARGET_PLATFORM} $<CONFIG>
VERBATIM
)
add_custom_target(flutter_assemble DEPENDS
Expand Down
Loading

0 comments on commit dfa1877

Please sign in to comment.