diff options
author | Henning Baldersheim <balder@oath.com> | 2018-08-31 14:41:39 +0200 |
---|---|---|
committer | Henning Baldersheim <balder@oath.com> | 2018-08-31 14:41:39 +0200 |
commit | fcb7879d82904fd86a6773c3b22bba2c31fd5b94 (patch) | |
tree | ee6b632ad163748417c954de6441455f2ede9bf9 | |
parent | 9724fa1edc8cab44d7ecc652ec36b4c859fabe31 (diff) |
Remove use and implementation of directwrite option
-rw-r--r-- | fnet/src/tests/info/info.cpp | 4 | ||||
-rw-r--r-- | fnet/src/vespa/fnet/config.cpp | 3 | ||||
-rw-r--r-- | fnet/src/vespa/fnet/config.h | 1 | ||||
-rw-r--r-- | fnet/src/vespa/fnet/connection.cpp | 45 | ||||
-rw-r--r-- | fnet/src/vespa/fnet/connection.h | 4 | ||||
-rw-r--r-- | fnet/src/vespa/fnet/iocomponent.cpp | 4 | ||||
-rw-r--r-- | fnet/src/vespa/fnet/iocomponent.h | 47 | ||||
-rw-r--r-- | fnet/src/vespa/fnet/transport.cpp | 8 | ||||
-rw-r--r-- | fnet/src/vespa/fnet/transport.h | 8 | ||||
-rw-r--r-- | fnet/src/vespa/fnet/transport_thread.cpp | 6 | ||||
-rw-r--r-- | fnet/src/vespa/fnet/transport_thread.h | 12 | ||||
-rw-r--r-- | jrt_test/src/jrt-test/simpleserver/simpleserver.cpp | 7 | ||||
-rw-r--r-- | messagebus/src/vespa/messagebus/network/rpcnetwork.cpp | 1 | ||||
-rw-r--r-- | searchcore/src/vespa/searchcore/fdispatch/program/fdispatch.cpp | 2 | ||||
-rw-r--r-- | searchlib/src/vespa/searchlib/engine/transportserver.h | 7 |
15 files changed, 17 insertions, 142 deletions
diff --git a/fnet/src/tests/info/info.cpp b/fnet/src/tests/info/info.cpp index cd0364cad6f..f22c1402437 100644 --- a/fnet/src/tests/info/info.cpp +++ b/fnet/src/tests/info/info.cpp @@ -70,10 +70,10 @@ TEST("info") { TEST("size of important objects") { - EXPECT_EQUAL(184u, sizeof(FNET_IOComponent)); + EXPECT_EQUAL(176u, sizeof(FNET_IOComponent)); EXPECT_EQUAL(32u, sizeof(FNET_Channel)); EXPECT_EQUAL(40u, sizeof(FNET_PacketQueue_NoLock)); - EXPECT_EQUAL(488u, sizeof(FNET_Connection)); + EXPECT_EQUAL(480u, sizeof(FNET_Connection)); EXPECT_EQUAL(48u, sizeof(std::condition_variable)); EXPECT_EQUAL(56u, sizeof(FNET_DataBuffer)); EXPECT_EQUAL(24u, sizeof(FastOS_Time)); diff --git a/fnet/src/vespa/fnet/config.cpp b/fnet/src/vespa/fnet/config.cpp index 01bc76791de..feed7f2d241 100644 --- a/fnet/src/vespa/fnet/config.cpp +++ b/fnet/src/vespa/fnet/config.cpp @@ -9,6 +9,5 @@ FNET_Config::FNET_Config() _maxInputBufferSize(0x10000), _maxOutputBufferSize(0x10000), _tcpNoDelay(true), - _logStats(false), - _directWrite(false) + _logStats(false) { } diff --git a/fnet/src/vespa/fnet/config.h b/fnet/src/vespa/fnet/config.h index e92540ce0bc..e94cf0f6105 100644 --- a/fnet/src/vespa/fnet/config.h +++ b/fnet/src/vespa/fnet/config.h @@ -18,7 +18,6 @@ public: uint32_t _maxOutputBufferSize; bool _tcpNoDelay; bool _logStats; - bool _directWrite; FNET_Config(); }; diff --git a/fnet/src/vespa/fnet/connection.cpp b/fnet/src/vespa/fnet/connection.cpp index 07086ca54a2..b9db8a46a4f 100644 --- a/fnet/src/vespa/fnet/connection.cpp +++ b/fnet/src/vespa/fnet/connection.cpp @@ -351,7 +351,7 @@ done_read: bool -FNET_Connection::Write(bool direct) +FNET_Connection::Write() { uint32_t my_write_work = 0; uint32_t writtenData = 0; // total data written @@ -437,29 +437,13 @@ FNET_Connection::Write(bool direct) } bool writePending = (_writeWork > 0); - if (direct) { // direct write (from post packet) - if (writtenData > 0) { - CountDirectDataWrite(writtenData); - CountDirectPacketWrite(writtenPackets); - } - if (writePending) { - AddRef_NoLock(); - guard.unlock(); - if (broken) { - Owner()->Close(this, /* needRef = */ false); - } else { - Owner()->EnableWrite(this, /* needRef = */ false); - } - } - } else { // normal write (from event loop) - guard.unlock(); - if (writtenData > 0) { - CountDataWrite(writtenData); - CountPacketWrite(writtenPackets); - } - if (!writePending) - EnableWriteEvent(false); + guard.unlock(); + if (writtenData > 0) { + CountDataWrite(writtenData); + CountPacketWrite(writtenPackets); } + if (!writePending) + EnableWriteEvent(false); return !broken; } @@ -701,16 +685,9 @@ FNET_Connection::PostPacket(FNET_Packet *packet, uint32_t chid) if (writeWork == 0 && !_flags._writeLock && _state == FNET_CONNECTED) { - if (GetConfig()->_directWrite) { - _flags._writeLock = true; - _queue.FlushPackets_NoLock(&_myQueue); - guard.unlock(); - Write(true); - } else { - AddRef_NoLock(); - guard.unlock(); - Owner()->EnableWrite(this, /* needRef = */ false); - } + AddRef_NoLock(); + guard.unlock(); + Owner()->EnableWrite(this, /* needRef = */ false); } return true; } @@ -805,7 +782,7 @@ FNET_Connection::HandleWriteEvent() _flags._writeLock = true; _queue.FlushPackets_NoLock(&_myQueue); } - broken = !Write(false); + broken = !Write(); break; case FNET_CLOSING: case FNET_CLOSED: diff --git a/fnet/src/vespa/fnet/connection.h b/fnet/src/vespa/fnet/connection.h index 120c675dc70..8e5e6280fab 100644 --- a/fnet/src/vespa/fnet/connection.h +++ b/fnet/src/vespa/fnet/connection.h @@ -227,10 +227,8 @@ private: * Write outgoing data to socket. * * @return false if socket is broken. - * @param direct is this a direct write (called directly from - * postpacket, without waiting for a write event) **/ - bool Write(bool direct); + bool Write(); bool writePendingAfterConnect(); public: diff --git a/fnet/src/vespa/fnet/iocomponent.cpp b/fnet/src/vespa/fnet/iocomponent.cpp index 8276da57e2e..148dabf5c60 100644 --- a/fnet/src/vespa/fnet/iocomponent.cpp +++ b/fnet/src/vespa/fnet/iocomponent.cpp @@ -20,9 +20,7 @@ FNET_IOComponent::FNET_IOComponent(FNET_TransportThread *owner, _ioc_timestamp(fastos::ClockSystem::now()), _ioc_lock(), _ioc_cond(), - _ioc_refcnt(1), - _ioc_directPacketWriteCnt(0), - _ioc_directDataWriteCnt(0) + _ioc_refcnt(1) { _ioc_spec = strdup(spec); assert(_ioc_spec != nullptr); diff --git a/fnet/src/vespa/fnet/iocomponent.h b/fnet/src/vespa/fnet/iocomponent.h index a48930428c7..16ecce2e345 100644 --- a/fnet/src/vespa/fnet/iocomponent.h +++ b/fnet/src/vespa/fnet/iocomponent.h @@ -55,10 +55,6 @@ protected: std::condition_variable _ioc_cond; // synchronization uint32_t _ioc_refcnt; // reference counter - // direct write stats kept locally - uint32_t _ioc_directPacketWriteCnt; - uint32_t _ioc_directDataWriteCnt; - public: /** @@ -180,19 +176,6 @@ public: /** - * Count direct packet write(s). This method will increase an - * internal counter. The shared stat counters may not be used - * because this method may be called by other threads than the - * transport thread. Note: The IO Component should be locked when - * this method is called. - * - * @param cnt the number of packets written (default is 1). - **/ - void CountDirectPacketWrite(uint32_t cnt = 1) - { _ioc_directPacketWriteCnt += cnt; } - - - /** * Count read data. This is a proxy method updating the stat * counters associated with the owning transport object. * @@ -213,36 +196,6 @@ public: /** - * Count direct written data. This method will increase an - * internal counter. The shared stat counters may not be used - * because this method may be called by other threads than the - * transport thread. Note: The IO Component should be locked when - * this method is called. - * - * @param bytes the number of bytes written. - **/ - void CountDirectDataWrite(uint32_t bytes) - { _ioc_directDataWriteCnt += bytes; } - - - /** - * Transfer the direct write stats held by this IO Component over to - * the stat counters associated with the owning transport object - * (and reset the local counters). Note: This method should only be - * called from the transport thread while having the lock on this IO - * Component. Note: This method is called from the transport loop - * and should generally not be called by application code. - **/ - void FlushDirectWriteStats() - { - _ioc_counters->CountPacketWrite(_ioc_directPacketWriteCnt); - _ioc_counters->CountDataWrite(_ioc_directDataWriteCnt); - _ioc_directPacketWriteCnt = 0; - _ioc_directDataWriteCnt = 0; - } - - - /** * Attach an event selector to this component. Before deleting an * IOC, one must first call detach_selector to detach the * selector. diff --git a/fnet/src/vespa/fnet/transport.cpp b/fnet/src/vespa/fnet/transport.cpp index 0a484f9caaa..55c847682c4 100644 --- a/fnet/src/vespa/fnet/transport.cpp +++ b/fnet/src/vespa/fnet/transport.cpp @@ -112,14 +112,6 @@ FNET_Transport::SetMaxOutputBufferSize(uint32_t bytes) } void -FNET_Transport::SetDirectWrite(bool directWrite) -{ - for (const auto &thread: _threads) { - thread->SetDirectWrite(directWrite); - } -} - -void FNET_Transport::SetTCPNoDelay(bool noDelay) { for (const auto &thread: _threads) { diff --git a/fnet/src/vespa/fnet/transport.h b/fnet/src/vespa/fnet/transport.h index a9c9eee2296..dbf914798fd 100644 --- a/fnet/src/vespa/fnet/transport.h +++ b/fnet/src/vespa/fnet/transport.h @@ -187,14 +187,6 @@ public: void SetMaxOutputBufferSize(uint32_t bytes); /** - * Enable or disable the direct write optimization. This is - * enabled by default and favors low latency above throughput. - * - * @param directWrite enable direct write? - **/ - void SetDirectWrite(bool directWrite); - - /** * Enable or disable use of the TCP_NODELAY flag with sockets * created by this transport object. * diff --git a/fnet/src/vespa/fnet/transport_thread.cpp b/fnet/src/vespa/fnet/transport_thread.cpp index 443c90f1af4..34ab9091072 100644 --- a/fnet/src/vespa/fnet/transport_thread.cpp +++ b/fnet/src/vespa/fnet/transport_thread.cpp @@ -166,12 +166,6 @@ FNET_TransportThread::UpdateStats() _now.SetNow(); // trade some overhead for better stats double ms = _now.MilliSecs() - _statTime.MilliSecs(); _statTime = _now; - for (FNET_IOComponent *comp = _componentsHead; - comp != nullptr; comp = comp->_ioc_next) - { - auto guard(comp->getGuard()); - comp->FlushDirectWriteStats(); - } { std::lock_guard<std::mutex> guard(_lock); _stats.Update(&_counters, ms / 1000.0); diff --git a/fnet/src/vespa/fnet/transport_thread.h b/fnet/src/vespa/fnet/transport_thread.h index 8af0642eebe..3e5fe49e73a 100644 --- a/fnet/src/vespa/fnet/transport_thread.h +++ b/fnet/src/vespa/fnet/transport_thread.h @@ -345,18 +345,6 @@ public: void SetMaxOutputBufferSize(uint32_t bytes) { _config._maxOutputBufferSize = bytes; } - - /** - * Enable or disable the direct write optimization. This is - * enabled by default and favors low latency above throughput. - * - * @param directWrite enable direct write? - **/ - void SetDirectWrite(bool directWrite) { - _config._directWrite = directWrite; - } - - /** * Enable or disable use of the TCP_NODELAY flag with sockets * created by this transport object. diff --git a/jrt_test/src/jrt-test/simpleserver/simpleserver.cpp b/jrt_test/src/jrt-test/simpleserver/simpleserver.cpp index ed7ff0e40bc..a0781ee4720 100644 --- a/jrt_test/src/jrt-test/simpleserver/simpleserver.cpp +++ b/jrt_test/src/jrt-test/simpleserver/simpleserver.cpp @@ -76,15 +76,10 @@ int App::Main() { if (_argc < 2) { - printf("usage: %s <listenspec> [ddw]\n", _argv[0]); - printf(" ddw = disable direct write\n"); + printf("usage: %s <listenspec>\n", _argv[0]); return 1; } FRT_Supervisor orb; - if (_argc >= 3 && strcmp(_argv[2], "ddw") == 0) { - printf("(direct write disabled)\n"); - orb.GetTransport()->SetDirectWrite(false); - } Server server(&orb); orb.Listen(_argv[1]); orb.Main(); diff --git a/messagebus/src/vespa/messagebus/network/rpcnetwork.cpp b/messagebus/src/vespa/messagebus/network/rpcnetwork.cpp index 108b94070bf..913338785f2 100644 --- a/messagebus/src/vespa/messagebus/network/rpcnetwork.cpp +++ b/messagebus/src/vespa/messagebus/network/rpcnetwork.cpp @@ -128,7 +128,6 @@ RPCNetwork::RPCNetwork(const RPCNetworkParams ¶ms) : _allowDispatchForEncode(params.getDispatchOnEncode()), _allowDispatchForDecode(params.getDispatchOnDecode()) { - _transport->SetDirectWrite(false); _transport->SetMaxInputBufferSize(params.getMaxInputBufferSize()); _transport->SetMaxOutputBufferSize(params.getMaxOutputBufferSize()); } diff --git a/searchcore/src/vespa/searchcore/fdispatch/program/fdispatch.cpp b/searchcore/src/vespa/searchcore/fdispatch/program/fdispatch.cpp index 93680a95d44..388d817e596 100644 --- a/searchcore/src/vespa/searchcore/fdispatch/program/fdispatch.cpp +++ b/searchcore/src/vespa/searchcore/fdispatch/program/fdispatch.cpp @@ -340,7 +340,6 @@ Fdispatch::Init() _nodeManager = std::make_unique<FastS_NodeManager>(_componentConfig, this, _partition); GetFNETTransport()->SetTCPNoDelay(_config->transportnodelay); - GetFNETTransport()->SetDirectWrite(_config->transportdirectwrite); if (ptportnum == 0) { throw vespalib::IllegalArgumentException("fdispatchrc.ptportnum must be non-zero, most likely an issue with config delivery."); @@ -349,7 +348,6 @@ Fdispatch::Init() _engineAdapter = std::make_unique<fdispatch::EngineAdapter>(this, _mypool.get()); _transportServer = std::make_unique<TransportServer>(*_engineAdapter, *_engineAdapter, *_engineAdapter, ptportnum, search::engine::TransportServer::DEBUG_ALL); _transportServer->setTCPNoDelay(_config->transportnodelay); - _transportServer->setDirectWrite(_config->transportdirectwrite); if (!_transportServer->start()) { _transportServer.reset(); diff --git a/searchlib/src/vespa/searchlib/engine/transportserver.h b/searchlib/src/vespa/searchlib/engine/transportserver.h index 691f6fbe791..67d373d5940 100644 --- a/searchlib/src/vespa/searchlib/engine/transportserver.h +++ b/searchlib/src/vespa/searchlib/engine/transportserver.h @@ -301,13 +301,6 @@ public: void setTCPNoDelay(bool noDelay) { _transport.SetTCPNoDelay(noDelay); } /** - * Enable or disable the use of a Q for throughput between search thread and network thread. - * - * @param directWrite bypasses Q - **/ - void setDirectWrite(bool directWrite) { _transport.SetDirectWrite(directWrite); } - - /** * Set a limit on how long a connection may be idle before closing it. * * @param millisecs max idle time in milliseconds |