[llvm] r371245 - [ORC] Make sure RPC channel-send is called in blocking calls and responses.
Lang Hames via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 6 13:47:06 PDT 2019
Hi Teresa,
Sorry -- I'm guessing this is a missing #include <atomic>. Will have a
patch in shortly.
Cheers,
Lang.
On Fri, Sep 6, 2019 at 1:25 PM Teresa Johnson <tejohnson at google.com> wrote:
> Hi Lang,
>
> I'm getting a lot of build errors that look to be related to this patch.
> Building with clang 4.0 on Linux x86:
>
> In file included from unittests/ExecutionEngine/Orc/QueueChannel.cpp:9:
> unittests/ExecutionEngine/Orc/QueueChannel.h:154:23: error: implicit
> instantiation of undefined template 'std::atomic<unsigned long>'
> std::atomic<size_t> InFlightIncomingMessages{0};
> ^
> /usr/bin/../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/atomic_base.h:126:12:
> note: template is declared here
> struct atomic;
> ^
> In file included from unittests/ExecutionEngine/Orc/QueueChannel.cpp:9:
> unittests/ExecutionEngine/Orc/QueueChannel.h:155:23: error: implicit
> instantiation of undefined template 'std::atomic<unsigned long>'
> std::atomic<size_t> CompletedIncomingMessages{0};
> ^
> /usr/bin/../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/atomic_base.h:126:12:
> note: template is declared here
> struct atomic;
> ^
> In file included from unittests/ExecutionEngine/Orc/QueueChannel.cpp:9:
> unittests/ExecutionEngine/Orc/QueueChannel.h:156:23: error: implicit
> instantiation of undefined template 'std::atomic<unsigned long>'
> std::atomic<size_t> InFlightOutgoingMessages{0};
> ^
> /usr/bin/../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/atomic_base.h:126:12:
> note: template is declared here
> struct atomic;
> ^
> In file included from unittests/ExecutionEngine/Orc/QueueChannel.cpp:9:
> unittests/ExecutionEngine/Orc/QueueChannel.h:157:23: error: implicit
> instantiation of undefined template 'std::atomic<unsigned long>'
> std::atomic<size_t> CompletedOutgoingMessages{0};
> ^
> /usr/bin/../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/atomic_base.h:126:12:
> note: template is declared here
> struct atomic;
> ^
> In file included from unittests/ExecutionEngine/Orc/QueueChannel.cpp:9:
> unittests/ExecutionEngine/Orc/QueueChannel.h:158:23: error: implicit
> instantiation of undefined template 'std::atomic<unsigned long>'
> std::atomic<size_t> SendCalls{0};
> ^
> /usr/bin/../lib/gcc/x86_64-linux-gnu/8/../../../../include/c++/8/bits/atomic_base.h:126:12:
> note: template is declared here
> struct atomic;
> ^
> In file included from unittests/ExecutionEngine/Orc/QueueChannel.cpp:9:
> unittests/ExecutionEngine/Orc/QueueChannel.h:72:7: warning:
> 'llvm::QueueChannel' has virtual functions but non-virtual destructor
> [-Wnon-virtual-dtor]
> class QueueChannel : public orc::rpc::RawByteChannel {
> ^
> unittests/ExecutionEngine/Orc/QueueChannel.h:92:38: error: cannot
> initialize object parameter of type 'llvm::orc::rpc::RawByteChannel' with
> an expression of type 'llvm::QueueChannel'
> return orc::rpc::RawByteChannel::endSendMessage();
> ^~~~~~~~~~~~~~
> unittests/ExecutionEngine/Orc/QueueChannel.h:104:38: error: cannot
> initialize object parameter of type 'llvm::orc::rpc::RawByteChannel' with
> an expression of type 'llvm::QueueChannel'
> return orc::rpc::RawByteChannel::endReceiveMessage();
> ^~~~~~~~~~~~~~~~~
> 1 warning and 7 errors generated.
>
>
> Teresa
>
> On Fri, Sep 6, 2019 at 12:20 PM Lang Hames via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: lhames
>> Date: Fri Sep 6 12:21:59 2019
>> New Revision: 371245
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=371245&view=rev
>> Log:
>> [ORC] Make sure RPC channel-send is called in blocking calls and
>> responses.
>>
>> ORC-RPC batches calls by default, and the channel's send method must be
>> called
>> to transfer any buffered calls to the remote. The call to send was
>> missing on
>> responses and blocking calls in the SingleThreadedRPCEndpoint. This patch
>> adds
>> the necessary calls and modifies the RPC unit test to check for them.
>>
>> Modified:
>> llvm/trunk/include/llvm/ExecutionEngine/Orc/RPCUtils.h
>> llvm/trunk/unittests/ExecutionEngine/Orc/QueueChannel.h
>> llvm/trunk/unittests/ExecutionEngine/Orc/RPCUtilsTest.cpp
>>
>> Modified: llvm/trunk/include/llvm/ExecutionEngine/Orc/RPCUtils.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/RPCUtils.h?rev=371245&r1=371244&r2=371245&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/ExecutionEngine/Orc/RPCUtils.h (original)
>> +++ llvm/trunk/include/llvm/ExecutionEngine/Orc/RPCUtils.h Fri Sep 6
>> 12:21:59 2019
>> @@ -338,7 +338,9 @@ public:
>> return Err;
>>
>> // Close the response message.
>> - return C.endSendMessage();
>> + if (auto Err = C.endSendMessage())
>> + return Err;
>> + return C.send();
>> }
>>
>> template <typename ChannelT, typename FunctionIdT, typename
>> SequenceNumberT>
>> @@ -350,7 +352,9 @@ public:
>> return Err2;
>> if (auto Err2 = serializeSeq(C, std::move(Err)))
>> return Err2;
>> - return C.endSendMessage();
>> + if (auto Err2 = C.endSendMessage())
>> + return Err2;
>> + return C.send();
>> }
>>
>> };
>> @@ -378,8 +382,11 @@ public:
>> C,
>> *ResultOrErr))
>> return Err;
>>
>> - // Close the response message.
>> - return C.endSendMessage();
>> + // End the response message.
>> + if (auto Err = C.endSendMessage())
>> + return Err;
>> +
>> + return C.send();
>> }
>>
>> template <typename ChannelT, typename FunctionIdT, typename
>> SequenceNumberT>
>> @@ -389,7 +396,9 @@ public:
>> return Err;
>> if (auto Err2 = C.startSendMessage(ResponseId, SeqNo))
>> return Err2;
>> - return C.endSendMessage();
>> + if (auto Err2 = C.endSendMessage())
>> + return Err2;
>> + return C.send();
>> }
>>
>> };
>> @@ -1518,6 +1527,12 @@ public:
>> detail::ResultTraits<typename Func::ReturnType>::consumeAbandoned(
>> std::move(Result));
>> return std::move(Err);
>> + }
>> +
>> + if (auto Err = this->C.send()) {
>> + detail::ResultTraits<typename Func::ReturnType>::consumeAbandoned(
>> + std::move(Result));
>> + return std::move(Err);
>> }
>>
>> while (!ReceivedResponse) {
>>
>> Modified: llvm/trunk/unittests/ExecutionEngine/Orc/QueueChannel.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/QueueChannel.h?rev=371245&r1=371244&r2=371245&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/unittests/ExecutionEngine/Orc/QueueChannel.h (original)
>> +++ llvm/trunk/unittests/ExecutionEngine/Orc/QueueChannel.h Fri Sep 6
>> 12:21:59 2019
>> @@ -80,6 +80,30 @@ public:
>> QueueChannel(QueueChannel&&) = delete;
>> QueueChannel& operator=(QueueChannel&&) = delete;
>>
>> + template <typename FunctionIdT, typename SequenceIdT>
>> + Error startSendMessage(const FunctionIdT &FnId, const SequenceIdT
>> &SeqNo) {
>> + ++InFlightOutgoingMessages;
>> + return orc::rpc::RawByteChannel::startSendMessage(FnId, SeqNo);
>> + }
>> +
>> + Error endSendMessage() {
>> + --InFlightOutgoingMessages;
>> + ++CompletedOutgoingMessages;
>> + return orc::rpc::RawByteChannel::endSendMessage();
>> + }
>> +
>> + template <typename FunctionIdT, typename SequenceNumberT>
>> + Error startReceiveMessage(FunctionIdT &FnId, SequenceNumberT &SeqNo) {
>> + ++InFlightIncomingMessages;
>> + return orc::rpc::RawByteChannel::startReceiveMessage(FnId, SeqNo);
>> + }
>> +
>> + Error endReceiveMessage() {
>> + --InFlightIncomingMessages;
>> + ++CompletedIncomingMessages;
>> + return orc::rpc::RawByteChannel::endReceiveMessage();
>> + }
>> +
>> Error readBytes(char *Dst, unsigned Size) override {
>> std::unique_lock<std::mutex> Lock(InQueue->getMutex());
>> while (Size) {
>> @@ -112,7 +136,10 @@ public:
>> return Error::success();
>> }
>>
>> - Error send() override { return Error::success(); }
>> + Error send() override {
>> + ++SendCalls;
>> + return Error::success();
>> + }
>>
>> void close() {
>> auto ChannelClosed = []() { return
>> make_error<QueueChannelClosedError>(); };
>> @@ -124,6 +151,11 @@ public:
>>
>> uint64_t NumWritten = 0;
>> uint64_t NumRead = 0;
>> + std::atomic<size_t> InFlightIncomingMessages{0};
>> + std::atomic<size_t> CompletedIncomingMessages{0};
>> + std::atomic<size_t> InFlightOutgoingMessages{0};
>> + std::atomic<size_t> CompletedOutgoingMessages{0};
>> + std::atomic<size_t> SendCalls{0};
>>
>> private:
>>
>>
>> Modified: llvm/trunk/unittests/ExecutionEngine/Orc/RPCUtilsTest.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/RPCUtilsTest.cpp?rev=371245&r1=371244&r2=371245&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/unittests/ExecutionEngine/Orc/RPCUtilsTest.cpp (original)
>> +++ llvm/trunk/unittests/ExecutionEngine/Orc/RPCUtilsTest.cpp Fri Sep 6
>> 12:21:59 2019
>> @@ -214,6 +214,17 @@ TEST(DummyRPC, TestCallAsyncVoidBool) {
>> EXPECT_FALSE(!!Err) << "Client failed to handle response from
>> void(bool)";
>> }
>>
>> + // The client should have made two calls to send: One implicit call to
>> + // negotiate the VoidBool function key, and a second to make the
>> VoidBool
>> + // call.
>> + EXPECT_EQ(Channels.first->SendCalls, 2U)
>> + << "Expected one send call to have been made by client";
>> +
>> + // The server should have made two calls to send: One to send the
>> response to
>> + // the negotiate call, and another to send the response to the
>> VoidBool call.
>> + EXPECT_EQ(Channels.second->SendCalls, 2U)
>> + << "Expected two send calls to have been made by server";
>> +
>> ServerThread.join();
>> }
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
>
> --
> Teresa Johnson | Software Engineer | tejohnson at google.com |
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190906/31bc57ab/attachment-0001.html>
More information about the llvm-commits
mailing list