optimize chained buffer a bit and factor out nop into non_owning_handle

include/libtorrent/Makefile.am

@@ -185,6 +185,7 @@ nobase_include_HEADERS = \
   aux_/scope_end.hpp                \
   aux_/vector.hpp                   \
   aux_/win_util.hpp                 \
+  aux_/non_owning_handle.hpp        \
   \
   extensions/smart_ban.hpp          \
   extensions/ut_metadata.hpp        \

include/libtorrent/aux_/non_owning_handle.hpp

@@ -0,0 +1,50 @@
+/*
+
+Copyright (c) 2016, Arvid Norberg
+All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions
+are met:
+
+    * Redistributions of source code must retain the above copyright
+      notice, this list of conditions and the following disclaimer.
+    * Redistributions in binary form must reproduce the above copyright
+      notice, this list of conditions and the following disclaimer in
+      the documentation and/or other materials provided with the distribution.
+    * Neither the name of the author nor the names of its
+      contributors may be used to endorse or promote products derived
+      from this software without specific prior written permission.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
+LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+POSSIBILITY OF SUCH DAMAGE.
+
+*/
+
+#ifndef TORRENT_NON_OWNING_HANDLE_HPP_INCLUDED
+#define TORRENT_NON_OWNING_HANDLE_HPP_INCLUDED
+
+namespace libtorrent { namespace aux {
+
+	// internal
+	struct non_owning_handle
+	{
+		explicit non_owning_handle(char* b) : m_buf(b) {}
+		char* get() const { return m_buf; }
+	private:
+		char* m_buf;
+	};
+
+}}
+
+#endif
+

include/libtorrent/chained_buffer.hpp

@@ -46,8 +46,8 @@ POSSIBILITY OF SUCH DAMAGE.
 #include "libtorrent/aux_/disable_warnings_pop.hpp"
 
 #ifdef _MSC_VER
-// visual studio requires the value in a deque to be copyable or movable. C++11
-// only requires that when calling functions with those requirements
+// visual studio requires the value in a deque to be copyable. C++11
+// has looser requirements depending on which functions are actually used.
 #define TORRENT_CPP98_DEQUE 1
 #else
 #define TORRENT_CPP98_DEQUE 0
@@ -81,7 +81,6 @@ namespace libtorrent
 				destruct_holder = rhs.destruct_holder;
 				move_holder = rhs.move_holder;
 				buf = rhs.buf;
-				start = rhs.start;
 				size = rhs.size;
 				used_size = rhs.used_size;
 				move_holder(&holder, &rhs.holder);
@@ -92,7 +91,6 @@ namespace libtorrent
 				destruct_holder = rhs.destruct_holder;
 				move_holder = rhs.move_holder;
 				buf = rhs.buf;
-				start = rhs.start;
 				size = rhs.size;
 				used_size = rhs.used_size;
 				move_holder(&holder, &rhs.holder);
@@ -114,11 +112,7 @@ namespace libtorrent
 			move_construct_holder_fun move_holder;
 #endif
 			std::aligned_storage<32>::type holder;
-			// TODO: 2 the pointers here should probably be const, since
-			// they're not supposed to be mutated once inserted into the send
-			// buffer
 			char* buf; // the first byte of the buffer
-			char* start; // the first byte to send/receive in the buffer
 			int size; // the total size of the buffer
 			int used_size; // this is the number of bytes to send/receive
 		};
@@ -131,7 +125,6 @@ namespace libtorrent
 
 		void pop_front(int bytes_to_pop);
 
-		//TODO: 3 use span<> instead of (buffer,s)
 		template <typename Holder>
 		void append_buffer(Holder buffer, int s, int used_size)
 		{
@@ -183,7 +176,6 @@ namespace libtorrent
 
 			b.buf = buffer.get();
 			b.size = s;
-			b.start = buffer.get();
 			b.used_size = used_size;
 
 #ifdef _MSC_VER

include/libtorrent/peer_connection.hpp

@@ -145,15 +145,6 @@ namespace libtorrent
 		torrent_peer* peerinfo;
 	};
 
-	// internal
-	struct nop
-	{
-		explicit nop(char* b) : m_buf(b) {}
-		char* get() const { return m_buf; }
-	private:
-		char* m_buf;
-	};
-
 	struct TORRENT_EXTRA_EXPORT peer_connection_hot_members
 	{
 		// if tor is set, this is an outgoing connection

src/chained_buffer.cpp

@@ -47,8 +47,10 @@ namespace libtorrent
 			buffer_t& b = m_vec.front();
 			if (b.used_size > bytes_to_pop)
 			{
-				b.start += bytes_to_pop;
+				b.buf += bytes_to_pop;
 				b.used_size -= bytes_to_pop;
+				b.size -= bytes_to_pop;
+				m_capacity -= bytes_to_pop;
 				m_bytes -= bytes_to_pop;
 				TORRENT_ASSERT(m_bytes <= m_capacity);
 				TORRENT_ASSERT(m_bytes >= 0);
@@ -75,9 +77,8 @@ namespace libtorrent
 		TORRENT_ASSERT(!m_destructed);
 		if (m_vec.empty()) return 0;
 		buffer_t& b = m_vec.back();
-		TORRENT_ASSERT(b.start != nullptr);
 		TORRENT_ASSERT(b.buf != nullptr);
-		return b.size - b.used_size - int(b.start - b.buf);
+		return b.size - b.used_size;
 	}
 
 	// tries to copy the given buffer to the end of the
@@ -102,9 +103,8 @@ namespace libtorrent
 		TORRENT_ASSERT(!m_destructed);
 		if (m_vec.empty()) return nullptr;
 		buffer_t& b = m_vec.back();
-		TORRENT_ASSERT(b.start != nullptr);
 		TORRENT_ASSERT(b.buf != nullptr);
-		char* const insert = b.start + b.used_size;
+		char* const insert = b.buf + b.used_size;
 		if (insert + s > b.buf + b.size) return nullptr;
 		b.used_size += s;
 		m_bytes += s;
@@ -133,16 +133,15 @@ namespace libtorrent
 		TORRENT_ASSERT(!m_destructed);
 		for (auto i = m_vec.begin(), end(m_vec.end()); bytes > 0 && i != end; ++i)
 		{
-			TORRENT_ASSERT(i->start != nullptr);
 			TORRENT_ASSERT(i->buf != nullptr);
 			if (i->used_size > bytes)
 			{
 				TORRENT_ASSERT(bytes > 0);
-				vec.push_back(Buffer(i->start, bytes));
+				vec.push_back(Buffer(i->buf, bytes));
 				break;
 			}
 			TORRENT_ASSERT(i->used_size > 0);
-			vec.push_back(Buffer(i->start, i->used_size));
+			vec.push_back(Buffer(i->buf, i->used_size));
 			bytes -= i->used_size;
 		}
 	}

src/peer_connection.cpp

@@ -64,6 +64,7 @@ POSSIBILITY OF SUCH DAMAGE.
 #include "libtorrent/close_reason.hpp"
 #include "libtorrent/aux_/has_block.hpp"
 #include "libtorrent/aux_/time.hpp"
+#include "libtorrent/aux_/non_owning_handle.hpp"
 
 #if TORRENT_USE_ASSERTS
 #include <set>
@@ -5447,7 +5448,7 @@ namespace libtorrent
 				// this const_cast is a here because chained_buffer need to be
 				// fixed.
 				char* ptr = const_cast<char*>(i->data());
-				m_send_buffer.prepend_buffer(nop(ptr), size, size);
+				m_send_buffer.prepend_buffer(aux::non_owning_handle(ptr), size, size);
 			}
 			set_send_barrier(next_barrier);
 		}

src/ut_metadata.cpp

@@ -52,6 +52,7 @@ POSSIBILITY OF SUCH DAMAGE.
 #include "libtorrent/io.hpp"
 #include "libtorrent/performance_counters.hpp" // for counters
 #include "libtorrent/aux_/time.hpp"
+#include "libtorrent/aux_/non_owning_handle.hpp"
 
 namespace libtorrent { namespace
 {
@@ -280,7 +281,7 @@ namespace libtorrent { namespace
 			// TODO: we really need to increment the refcounter on the torrent
 			// while this buffer is still in the peer's send buffer
 			if (metadata_piece_size) m_pc.append_const_send_buffer(
-				nop(const_cast<char*>(metadata)), metadata_piece_size);
+				aux::non_owning_handle(const_cast<char*>(metadata)), metadata_piece_size);
 
 			m_pc.stats_counters().inc_stats_counter(counters::num_outgoing_extended);
 			m_pc.stats_counters().inc_stats_counter(counters::num_outgoing_metadata);

test/test_buffer.cpp

@@ -220,25 +220,25 @@ TORRENT_TEST(chained_buffer)
 		b.append_buffer(holder(b1), 512, 6);
 		TEST_EQUAL(buffer_list.size(), 1);
 
-		TEST_CHECK(b.capacity() == 512);
-		TEST_CHECK(b.size() == 6);
+		TEST_EQUAL(b.capacity(), 512);
+		TEST_EQUAL(b.size(), 6);
 		TEST_CHECK(!b.empty());
-		TEST_CHECK(b.space_in_last_buffer() == 512 - 6);
+		TEST_EQUAL(b.space_in_last_buffer(), 512 - 6);
 
 		b.pop_front(3);
 
-		TEST_CHECK(b.capacity() == 512);
-		TEST_CHECK(b.size() == 3);
+		TEST_EQUAL(b.capacity(), 512 - 3);
+		TEST_EQUAL(b.size(), 3);
 		TEST_CHECK(!b.empty());
-		TEST_CHECK(b.space_in_last_buffer() == 512 - 6);
+		TEST_EQUAL(b.space_in_last_buffer(), 512 - 6);
 
 		bool ret = b.append(data, 6) != nullptr;
 
 		TEST_CHECK(ret == true);
-		TEST_CHECK(b.capacity() == 512);
-		TEST_CHECK(b.size() == 9);
+		TEST_EQUAL(b.capacity(), 512 - 3);
+		TEST_EQUAL(b.size(), 9);
 		TEST_CHECK(!b.empty());
-		TEST_CHECK(b.space_in_last_buffer() == 512 - 12);
+		TEST_EQUAL(b.space_in_last_buffer(), 512 - 12);
 
 		char data2[1024];
 		ret = b.append(data2, 1024) != nullptr;
@@ -248,17 +248,17 @@ TORRENT_TEST(chained_buffer)
 		char* b2 = allocate_buffer(512);
 		std::memcpy(b2, data, 6);
 		b.append_buffer(holder(b2), 512, 6);
-		TEST_CHECK(buffer_list.size() == 2);
+		TEST_EQUAL(buffer_list.size(), 2);
 
 		char* b3 = allocate_buffer(512);
 		std::memcpy(b3, data, 6);
 		b.append_buffer(holder(b3), 512, 6);
-		TEST_CHECK(buffer_list.size() == 3);
+		TEST_EQUAL(buffer_list.size(), 3);
 
-		TEST_CHECK(b.capacity() == 512 * 3);
-		TEST_CHECK(b.size() == 21);
+		TEST_EQUAL(b.capacity(), 512 * 3 - 3);
+		TEST_EQUAL(b.size(), 21);
 		TEST_CHECK(!b.empty());
-		TEST_CHECK(b.space_in_last_buffer() == 512 - 6);
+		TEST_EQUAL(b.space_in_last_buffer(), 512 - 6);
 
 		TEST_CHECK(compare_chained_buffer(b, "barfoobar", 9));
 
@@ -268,10 +268,10 @@ TORRENT_TEST(chained_buffer)
 		b.pop_front(5 + 6);
 
 		TEST_CHECK(buffer_list.size() == 2);
-		TEST_CHECK(b.capacity() == 512 * 2);
-		TEST_CHECK(b.size() == 10);
+		TEST_EQUAL(b.capacity(), 512 * 2 - 2);
+		TEST_EQUAL(b.size(), 10);
 		TEST_CHECK(!b.empty());
-		TEST_CHECK(b.space_in_last_buffer() == 512 - 6);
+		TEST_EQUAL(b.space_in_last_buffer(), 512 - 6);
 
 		char const* str = "obarfooba";
 		TEST_CHECK(compare_chained_buffer(b, str, 9));