Skip to content

Commit aeac2ad

Browse files
MiguelCompanyMiguelCompany
authored
Refactor output arguments on RPC interfaces (#478)
* Refs #23232. Avoid 100% CPU usage in server's `run()` method. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #23232. Output arguments moved into result. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #23232. Fix client source code. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #23232. Fix CdrAuxHeaderImpl. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #23232. Header only includes output structures when necessary. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #23232. Changes in server side. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #23232. Avoid nested structures (unsupported by Swig). Signed-off-by: Miguel Company <miguelcompany@eprosima.com> * Refs #23232. Address review comments. Signed-off-by: Miguel Company <miguelcompany@eprosima.com> --------- Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
1 parent e180325 commit aeac2ad

6 files changed

Lines changed: 81 additions & 32 deletions

File tree

‎src/main/java/com/eprosima/fastcdr/idl/templates/TypesHeader.stg‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,12 @@ private:
189189
>>
190190

191191
interface(ctx, parent, interface, export_list) ::= <<
192+
193+
namespace detail {
194+
195+
$interface.operations : { op | $if(op.outputparam)$$operation_out_struct_fwd_decl(interface, op)$$endif$}$
196+
}
197+
192198
/*!
193199
* @brief This class represents the interface $interface.name$ defined by the user in the IDL file.
194200
* @ingroup $ctx.trimfilename$
@@ -200,11 +206,17 @@ public:
200206

201207
$export_list$
202208
};
209+
210+
namespace detail {
211+
212+
$interface.operations : { op | $if(op.outputparam)$$operation_out_struct(interface, op)$$"\n"$$endif$}$
213+
}
203214
>>
204215

205216
operation(ctx, parent, operation, param_list, operation_type) ::= <<
217+
206218
virtual $operationRetType(operation)$ $operation.name$(
207-
$paramDeclarations(params=operation.parameters)$) = 0;
219+
$paramDeclarations(params=operation.inputparam)$) = 0;
208220

209221
>>
210222

@@ -939,11 +951,37 @@ $endif$
939951
operationRetType(operation) ::= <%
940952
$if(operation.annotationFeed)$
941953
std::shared_ptr<eprosima::fastdds::dds::rpc::RpcClientReader<$paramRetType(operation.rettype)$> >
954+
$elseif(operation.outputparam)$
955+
eprosima::fastdds::dds::rpc::RpcFuture<$paramRetType(operation.outTypeCode)$>
942956
$else$
943957
eprosima::fastdds::dds::rpc::RpcFuture<$paramRetType(operation.rettype)$>
944958
$endif$
945959
%>
946960

961+
operation_out_struct_fwd_decl(interface, operation) ::= <<
962+
struct $interface.name$_$operation.name$_Out;
963+
964+
>>
965+
966+
operation_out_struct(interface, operation) ::= <<
967+
struct $interface.name$_$operation.name$_Out
968+
{
969+
$if(operation.annotationFeed)$
970+
eprosima::fastcdr::optional<$operation.rettypeparam.typecode.cppTypename$\> $operation.rettypeparam.name$;
971+
eprosima::fastcdr::optional<bool\> finished_;
972+
$else$
973+
$if([operation.outputparam, operation.rettypeparam])$
974+
$[operation.outputparam, operation.rettypeparam]:{param | $parameter_declaration(param)$}; separator="\n"$
975+
$endif$
976+
$endif$
977+
};
978+
979+
>>
980+
981+
parameter_declaration(param) ::= <%
982+
$param.typecode.cppTypename$ $param.name$;
983+
%>
984+
947985
//{ Fast DDS-Gen extensions
948986
module_conversion(ctx, parent, modules, definition_list) ::= <<
949987
$modules : { module |

‎src/main/java/com/eprosima/fastdds/idl/templates/ClientSource.stg‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -363,17 +363,14 @@ operation_details(parent, operation) ::= <<
363363
public:
364364

365365
$operationRetType(operation)$ $operation.name$(
366-
$paramDeclarations(params=operation.parameters)$) override
366+
$paramDeclarations(params=operation.inputparam)$) override
367367
{
368368
$if(operation.annotationFeed)$
369369
// Create a reader to hold the result
370370
auto result = std::make_shared<$operation.name$_reader>(requester_);
371371
$else$
372372
// Create a promise to hold the result
373373
auto result = std::make_shared<$operation.name$_promise>();
374-
$if(operation.outputparam)$
375-
$operation.outputparam:{param | result->$param.name$ = &$param.name$;}; separator="\n"$
376-
$endif$
377374
$endif$
378375

379376
// Create and send the request
@@ -609,10 +606,7 @@ $if(operation.annotationFeed)$
609606
$else$
610607
struct $operation.name$_promise : public IReplyProcessor
611608
{
612-
std::promise<$paramRetType(operation.rettype)$> promise;
613-
$if(operation.outputparam)$
614-
$operation.outputparam:{param | $param.typecode.cppTypename$* $param.name$ = nullptr;}; separator="\n"$
615-
$endif$
609+
std::promise<$if(operation.outputparam)$$paramRetType(operation.outTypeCode)$$else$$paramRetType(operation.rettype)$$endif$> promise;
616610

617611
void process_reply(
618612
const ReplyType& reply,
@@ -643,8 +637,11 @@ $else$
643637
if (result.result.has_value())
644638
{
645639
const auto& out = result.result.value();
646-
$operation.outputparam:{param | *$param.name$ = out.$param.name$;}; separator="\n"$
640+
$if(operation.outputparam)$
641+
promise.set_value(out);
642+
$else$
647643
promise.set_value($if(operation.rettype)$out.return_$endif$);
644+
$endif$
648645
return;
649646
}
650647
$operation.exceptions : { exception |$operation_result_exception(name=[exception.formatedScopedname, "_ex"], receiver="promise")$}; separator="\n"$
@@ -666,7 +663,9 @@ $endif$
666663

667664
operationRetType(operation) ::= <%
668665
$if(operation.annotationFeed)$
669-
std::shared_ptr<eprosima::fastdds::dds::rpc::RpcClientReader<$paramRetType(operation.rettype)$>>
666+
std::shared_ptr<eprosima::fastdds::dds::rpc::RpcClientReader<$paramRetType(operation.rettype)$> >
667+
$elseif(operation.outputparam)$
668+
eprosima::fastdds::dds::rpc::RpcFuture<$paramRetType(operation.outTypeCode)$>
670669
$else$
671670
eprosima::fastdds::dds::rpc::RpcFuture<$paramRetType(operation.rettype)$>
672671
$endif$

‎src/main/java/com/eprosima/fastdds/idl/templates/InterfaceDetails.stg‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ $operation_in_struct(interface, operation)$
8383

8484
$operation.inputparam : { param | $if (param.annotationFeed)$$operation_feed_struct(interface, operation, param)$$endif$ }$
8585

86-
$operation_out_struct(interface, operation)$
86+
$if(!operation.outputparam)$$operation_out_struct(interface, operation)$$endif$
8787

8888
$operation_result_struct(interface, operation)$
8989

@@ -125,7 +125,7 @@ struct $interface.name$_$operation.name$_Out
125125
operation_result_struct(interface, operation) ::= <<
126126
struct $interface.name$_$operation.name$_Result
127127
{
128-
eprosima::fastcdr::optional<$interface.name$_$operation.name$_Out\> result;
128+
eprosima::fastcdr::optional<$operation.outTypeCode.cppTypename$\> result;
129129
$operation.exceptions : { exception |$operation_result_exception(typename=exception.scopedname, name=[exception.formatedScopedname, "_ex"])$}; separator="\n"$
130130
};
131131
>>

‎src/main/java/com/eprosima/fastdds/idl/templates/ServerHeader.stg‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,10 +130,18 @@ $if(op.parameters)$
130130
$endif$
131131
/*result*/ eprosima::fastdds::dds::rpc::RpcServerWriter<$paramRetType(op.rettype)$>& result_writer) = 0;
132132
$else$
133-
$if(op.parameters)$
133+
$if(op.outputparam)$
134+
virtual $paramRetType(op.outTypeCode)$ $op.name$(
135+
$if(op.inputparam)$
136+
const $interface.name$Server_ClientContext& info,
137+
$operation_parameters(op.inputparam)$) = 0;
138+
$else$
139+
const $interface.name$Server_ClientContext& info) = 0;
140+
$endif$
141+
$elseif(op.inputparam)$
134142
virtual $paramRetType(op.rettype)$ $op.name$(
135143
const $interface.name$Server_ClientContext& info,
136-
$operation_parameters(op.parameters)$) = 0;
144+
$operation_parameters(op.inputparam)$) = 0;
137145
$else$
138146
virtual $paramRetType(op.rettype)$ $op.name$(
139147
const $interface.name$Server_ClientContext& info) = 0;

‎src/main/java/com/eprosima/fastdds/idl/templates/ServerImplementation.stg‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,20 +66,25 @@ $if(op.parameters)$
6666
$endif$
6767
/*result*/ eprosima::fastdds::dds::rpc::RpcServerWriter<$paramRetType(op.rettype)$>& result_writer) override
6868
$else$
69-
$if(op.parameters)$
69+
$if(op.outputparam)$
70+
$paramRetType(op.outTypeCode)$ $op.name$(
71+
$else$
7072
$paramRetType(op.rettype)$ $op.name$(
73+
$endif$
74+
$if(op.inputparam)$
7175
const $interface.name$Server_ClientContext& info,
72-
$operation_parameters(op.parameters)$) override
76+
$operation_parameters(op.inputparam)$) override
7377
$else$
74-
$paramRetType(op.rettype)$ $op.name$(
7578
const $interface.name$Server_ClientContext& info) override
7679
$endif$
7780
$endif$
7881
{
7982
static_cast<void>(info);
80-
$op.parameters : {param | static_cast<void>($param.name$);}; anchor, separator="\n"$
8183
$if(op.annotationFeed)$
84+
$op.parameters : {param | static_cast<void>($param.name$);}; anchor, separator="\n"$
8285
static_cast<void>(result_writer);
86+
$else$
87+
$op.inputparam : {param | static_cast<void>($param.name$);}; anchor, separator="\n"$
8388
$endif$
8489
throw eprosima::fastdds::dds::rpc::RemoteUnsupportedError("Operation '$op.name$' is not implemented");
8590
}

‎src/main/java/com/eprosima/fastdds/idl/templates/ServerSource.stg‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,9 @@ public:
138138
finish_condition_.set_trigger_value(false);
139139
fdds::WaitSet waitset;
140140
waitset.attach_condition(finish_condition_);
141-
waitset.attach_condition(replier_->get_replier_reader()->get_statuscondition());
141+
fdds::StatusCondition& status_condition = replier_->get_replier_reader()->get_statuscondition();
142+
status_condition.set_enabled_statuses(fdds::StatusMask::data_available());
143+
waitset.attach_condition(status_condition);
142144

143145
while (true)
144146
{
@@ -752,18 +754,17 @@ $endif$
752754
reply.$op.name$->result->finished_ = true;
753755
replier_->send_reply(&reply, req->info);
754756
$else$
755-
$op.outputparam : {param | $param.typecode.cppTypename$ $param.name$ $if(param.onlyOutput)${\}$else$= req->request.$op.name$->$param.name$$endif$;}; separator="\n"$
756-
$if(op.rettype)$$op.rettype.cppTypename$ result = $else$/*void*/ $endif$implementation_->$op.name$(
757-
*req$if(op.parameters)$,$else$);$endif$
758-
$if(op.parameters)$
759-
$op.parameters : {param | $operation_call_parameter(op, param)$}; separator=",\n"$);
760-
$endif$
761757
ReplyType reply{};
762758
reply.$op.name$ = $op.resultTypeCode.scopedname${};
759+
$if(op.outputparam)$
760+
reply.$op.name$->result = implementation_->$op.name$(
761+
$else$
763762
reply.$op.name$->result = $op.outTypeCode.scopedname${};
764-
$op.outputparam : {param | reply.$op.name$->result->$param.name$ = $param.name$;}; separator="\n"$
765-
$if(op.rettype)$
766-
reply.$op.name$->result->return_ = result;
763+
$if(op.rettype)$reply.$op.name$->result->return_ = $else$/*void*/ $endif$implementation_->$op.name$(
764+
$endif$
765+
*req$if(op.inputparam)$,$else$);$endif$
766+
$if(op.inputparam)$
767+
$op.inputparam : {param | $operation_call_parameter(op, param)$}; separator=",\n"$);
767768
$endif$
768769
replier_->send_reply(&reply, req->info);
769770
$endif$
@@ -784,9 +785,7 @@ $endif$
784785
>>
785786

786787
operation_call_parameter(op, param) ::= <%
787-
$if(param.output)$
788-
$param.name$
789-
$elseif(param.annotationFeed)$
788+
$if(param.annotationFeed)$
790789
*(req->$op.name$_feeds.$param.name$)
791790
$else$
792791
req->request.$op.name$->$param.name$

0 commit comments

Comments
 (0)